Settings: Avoid static initialization fiasco

When building on macOS with clang 8 and ThreadSanitizer enabled,
memcached_testapp crashes when destructing the Settings object:

    libc++abi.dylib: terminating with uncaught exception of type std::__1::system_error: mutex lock failed: Invalid argument

With the following backtrace from where the system_error exception is thrown:

* thread couchbase#1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x00007fff7b6411f6 libc++abi.dylib` __cxa_throw
    frame couchbase#1: 0x00007fff7b6147af libc++.1.dylib` std::__1::__throw_system_error(int, char const*)  + 77
    frame couchbase#2: 0x00007fff7b606c93 libc++.1.dylib` std::__1::mutex::lock()  + 29
    frame couchbase#3: 0x00000001009c27ff memcached_testapp` folly::SharedMutexImpl<...>::annotateLazyCreate() [inlined] std::__1::unique_lock<std::__1::mutex>::unique_lock(__m=<unavailable>)  + 191 at __mutex_base:131
    frame couchbase#4: 0x00000001009c27f7 memcached_testapp` folly::SharedMutexImpl<...>::annotateLazyCreate() [inlined] std::__1::unique_lock<std::__1::mutex>::unique_lock(__m=<unavailable>)  at __mutex_base:131
  * frame couchbase#5: 0x00000001009c27f7 memcached_testapp` folly::SharedMutexImpl<...>::annotateLazyCreate()  + 140 at SharedMutex.cpp:33
    frame couchbase#6: 0x00000001009c276b memcached_testapp` folly::SharedMutexImpl<...>::annotateLazyCreate(this=0x0000000100c7ddc8)  + 43 at SharedMutex.h:705
    frame #7: 0x00000001009c191a memcached_testapp` folly::SharedMutexImpl<...>::~SharedMutexImpl() [inlined] folly::SharedMutexImpl<...>::annotateDestroy(this=<unavailable>)  + 8 at SharedMutex.h:718
    frame #8: 0x00000001009c1912 memcached_testapp` folly::SharedMutexImpl<...>::~SharedMutexImpl()  + 24 at SharedMutex.h:324
    frame #9: 0x00000001009c18fa memcached_testapp` folly::SharedMutexImpl<...>::~SharedMutexImpl(this=0x0000000100c7ddc8)  + 26 at SharedMutex.h:300
    frame #10: 0x000000010076b837 memcached_testapp` folly::Synchronized<...>::~Synchronized(this=0x0000000100c7ddb0)  + 55 at Synchronized.h:484
    frame #11: 0x000000010075d1d9 memcached_testapp` folly::Synchronized<...>::~Synchronized(this=0x0000000100c7ddb0)  + 41 at Synchronized.h:484
    frame #12: 0x000000010075e8fc memcached_testapp` Settings::~Settings(this=0x0000000100c7db10)  + 76 at settings.h:51
    frame #13: 0x000000010075c8b9 memcached_testapp` Settings::~Settings(this=0x0000000100c7db10)  + 41 at settings.h:51
    frame #14: 0x0000000102b7d5c1 libclang_rt.tsan_osx_dynamic.dylib` cxa_at_exit_wrapper(void*)  + 33
    frame #15: 0x00007fff7d72beed libsystem_c.dylib` __cxa_finalize_ranges  + 351
    frame #16: 0x00007fff7d72c1fe libsystem_c.dylib` exit  + 55
    frame #17: 0x00007fff7d67f01c libdyld.dylib` start  + 8

The problem is at frame 5, where as part of the destruction of
SharedMutex (as used by a member variable of Settings) we are
attempting to acquire a mutex which has already been destructed (as it
is itself a function-local static) - i.e. we have encountered the
static initialization order fiasco :(

Address this by changing `settings` to no longer be a plain static
(global) variable, and instead be created on first use via a new
Settings::instance() static method.

Change-Id: I40bd44ed0ae32eb55271ce739fdf990d9869c32f
Reviewed-on: http://review.couchbase.org/113640
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Jim Walker <jim@couchbase.com>

Author: daverigby

daemon/cmdline.cc

@@ -79,19 +79,19 @@ void parse_arguments(int argc, char** argv) {
         std::exit(EXIT_FAILURE);
     }
 
-    load_config_file(config_file.c_str(), settings);
+    load_config_file(config_file.c_str(), Settings::instance());
 
     if (verbosity > 0) {
-        settings.setVerbose(verbosity);
+        Settings::instance().setVerbose(verbosity);
     }
 
     if (threads > 0) {
-        settings.setNumWorkerThreads(threads);
+        Settings::instance().setNumWorkerThreads(threads);
     }
 
     if (requests > 0) {
-        settings.setRequestsPerEventNotification(requests,
-                                                 EventPriority::Default);
+        Settings::instance().setRequestsPerEventNotification(
+                requests, EventPriority::Default);
     }
 }
 

daemon/config_parse.cc

@@ -43,7 +43,7 @@ boost::optional<nlohmann::json> validate_proposed_config_changes(
     try {
         auto json = nlohmann::json::parse(new_cfg);
         Settings new_settings(json);
-        settings.updateSettings(new_settings, false);
+        Settings::instance().updateSettings(new_settings, false);
         return {};
     } catch (const std::exception& exception) {
         errors.push_back(exception.what());
@@ -58,7 +58,7 @@ void reload_config_file() {
     try {
         auto content = cb::io::loadFile(get_config_file());
         Settings new_settings(nlohmann::json::parse(content));
-        settings.updateSettings(new_settings, true);
+        Settings::instance().updateSettings(new_settings, true);
     } catch (const std::exception& exception) {
         LOG_WARNING(
                 "Validation failed while reloading config file '{}'. Error: {}",

daemon/connection.cc

@@ -357,7 +357,7 @@ cb::rbac::PrivilegeAccess Connection::checkPrivilege(
         const std::string privilege_string = cb::rbac::to_string(privilege);
         const std::string context = privilegeContext.to_string();
 
-        if (settings.isPrivilegeDebug()) {
+        if (Settings::instance().isPrivilegeDebug()) {
             audit_privilege_debug(*this,
                                   command,
                                   all_buckets[bucketIndex].name,
@@ -743,7 +743,8 @@ int Connection::sslPreConnection() {
             disconnect = true;
             break;
         case cb::x509::Status::NotPresent:
-            if (settings.getClientCertMode() == cb::x509::Mode::Mandatory) {
+            if (Settings::instance().getClientCertMode() ==
+                cb::x509::Mode::Mandatory) {
                 disconnect = true;
             } else if (is_default_bucket_enabled()) {
                 associate_bucket(*this, "default");
@@ -1122,7 +1123,7 @@ int Connection::sslRead(char* dest, size_t nbytes) {
 int Connection::sslWrite(const char* src, size_t nbytes) {
     int ret = 0;
 
-    int chunksize = settings.getBioDrainBufferSize();
+    int chunksize = Settings::instance().getBioDrainBufferSize();
 
     while (ret < int(nbytes)) {
         int n;
@@ -1249,7 +1250,7 @@ void Connection::ensureIovSpace() {
 
 bool Connection::enableSSL(const std::string& cert, const std::string& pkey) {
     if (ssl.enable(cert, pkey)) {
-        if (settings.getVerbose() > 1) {
+        if (Settings::instance().getVerbose() > 1) {
             ssl.dumpCipherList(getId());
         }
 
@@ -1267,7 +1268,7 @@ Connection::Connection(FrontEndThread& thr)
       peername("unknown"),
       sockname("unknown"),
       stateMachine(*this),
-      max_reqs_per_event(settings.getRequestsPerEventNotification(
+      max_reqs_per_event(Settings::instance().getRequestsPerEventNotification(
               EventPriority::Default)) {
     updateDescription();
     cookies.emplace_back(std::unique_ptr<Cookie>{new Cookie(*this)});
@@ -1286,7 +1287,7 @@ Connection::Connection(SOCKET sfd,
       peername(cb::net::getpeername(socketDescriptor)),
       sockname(cb::net::getsockname(socketDescriptor)),
       stateMachine(*this),
-      max_reqs_per_event(settings.getRequestsPerEventNotification(
+      max_reqs_per_event(Settings::instance().getRequestsPerEventNotification(
               EventPriority::Default)) {
     setTcpNoDelay(true);
     updateDescription();
@@ -1330,7 +1331,7 @@ void Connection::setState(StateMachine::State next_state) {
 }
 
 void Connection::runStateMachinery() {
-    if (settings.getVerbose() > 1) {
+    if (Settings::instance().getVerbose() > 1) {
         do {
             LOG_DEBUG("{} - Running task: {}",
                       getId(),
@@ -1557,15 +1558,18 @@ void Connection::setPriority(Connection::Priority priority) {
     switch (priority) {
     case Priority::High:
         max_reqs_per_event =
-                settings.getRequestsPerEventNotification(EventPriority::High);
+                Settings::instance().getRequestsPerEventNotification(
+                        EventPriority::High);
         return;
     case Priority::Medium:
         max_reqs_per_event =
-                settings.getRequestsPerEventNotification(EventPriority::Medium);
+                Settings::instance().getRequestsPerEventNotification(
+                        EventPriority::Medium);
         return;
     case Priority::Low:
         max_reqs_per_event =
-                settings.getRequestsPerEventNotification(EventPriority::Low);
+                Settings::instance().getRequestsPerEventNotification(
+                        EventPriority::Low);
         return;
     }
     throw std::invalid_argument("Unkown priority: " +
@@ -1575,9 +1579,10 @@ void Connection::setPriority(Connection::Priority priority) {
 bool Connection::selectedBucketIsXattrEnabled() const {
     auto* bucketEngine = getBucketEngine();
     if (bucketEngine) {
-        return settings.isXattrEnabled() && bucketEngine->isXattrEnabled();
+        return Settings::instance().isXattrEnabled() &&
+               bucketEngine->isXattrEnabled();
     }
-    return settings.isXattrEnabled();
+    return Settings::instance().isXattrEnabled();
 }
 
 ENGINE_ERROR_CODE Connection::add_packet_to_send_pipe(

daemon/connections.cc

@@ -180,7 +180,7 @@ Connection* conn_new(SOCKET sfd,
                  bucket.name);
     }
 
-    if (settings.getVerbose() > 1) {
+    if (Settings::instance().getVerbose() > 1) {
         LOG_DEBUG("<{} new client connection", sfd);
     }
 

daemon/cookie.cc

@@ -251,7 +251,7 @@ void Cookie::sendDynamicBuffer() {
 void Cookie::sendNotMyVBucket() {
     auto pair = connection.getBucket().clusterConfiguration.getConfiguration();
     if (pair.first == -1 || (pair.first == connection.getClustermapRevno() &&
-                             settings.isDedupeNmvbMaps())) {
+                             Settings::instance().isDedupeNmvbMaps())) {
         // We don't have a vbucket map, or we've already sent it to the
         // client
         mcbp_add_header(*this,
@@ -414,7 +414,7 @@ std::string Cookie::getPrintableRequestKey() const {
 }
 
 void Cookie::logCommand() const {
-    if (settings.getVerbose() == 0) {
+    if (Settings::instance().getVerbose() == 0) {
         // Info is not enabled.. we don't want to try to format
         // output
         return;
@@ -437,7 +437,7 @@ void Cookie::logResponse(const char* reason) const {
 }
 
 void Cookie::logResponse(ENGINE_ERROR_CODE code) const {
-    if (settings.getVerbose() == 0) {
+    if (Settings::instance().getVerbose() == 0) {
         // Info is not enabled.. we don't want to try to format
         // output
         return;

daemon/datatype.cc

@@ -21,11 +21,11 @@
 bool Datatype::isSupported(cb::mcbp::Feature feature) {
     switch (feature) {
     case cb::mcbp::Feature::XATTR:
-        return settings.isXattrEnabled();
+        return Settings::instance().isXattrEnabled();
     case cb::mcbp::Feature::JSON:
-        return settings.isDatatypeJsonEnabled();
+        return Settings::instance().isDatatypeJsonEnabled();
     case cb::mcbp::Feature::SNAPPY:
-        return settings.isDatatypeSnappyEnabled();
+        return Settings::instance().isDatatypeSnappyEnabled();
     case cb::mcbp::Feature::TLS:
     case cb::mcbp::Feature::TCPNODELAY:
     case cb::mcbp::Feature::MUTATION_SEQNO:

daemon/mcaudit.cc

@@ -335,8 +335,8 @@ static void event_state_listener(uint32_t id, bool enabled) {
 
 void initialize_audit() {
     /* Start the audit daemon */
-    auditHandle = cb::audit::create_audit_daemon(settings.getAuditFile(),
-                                                 get_server_api()->cookie);
+    auditHandle = cb::audit::create_audit_daemon(
+            Settings::instance().getAuditFile(), get_server_api()->cookie);
     if (!auditHandle) {
         FATAL_ERROR(EXIT_FAILURE, "FATAL: Failed to start audit daemon");
     }
@@ -349,7 +349,7 @@ void shutdown_audit() {
 }
 
 ENGINE_ERROR_CODE reconfigure_audit(Cookie& cookie) {
-    if (auditHandle->configure_auditdaemon(settings.getAuditFile(),
+    if (auditHandle->configure_auditdaemon(Settings::instance().getAuditFile(),
                                            static_cast<void*>(&cookie))) {
         return ENGINE_EWOULDBLOCK;
     }

daemon/mcbp.cc

@@ -114,7 +114,7 @@ void mcbp_add_header(Cookie& cookie,
                                       header.getOpaque(),
                                       cookie.getCas());
 
-    if (settings.getVerbose() > 1) {
+    if (Settings::instance().getVerbose() > 1) {
         auto* header = reinterpret_cast<const cb::mcbp::Header*>(wbuf.data());
         try {
             LOG_TRACE("<{} Sending: {}",

daemon/mcbp_executors.cc

@@ -216,7 +216,7 @@ static void verbosity_executor(Cookie& cookie) {
     if (level < 0 || level > MAX_VERBOSITY_LEVEL) {
         level = MAX_VERBOSITY_LEVEL;
     }
-    settings.setVerbose(level);
+    Settings::instance().setVerbose(level);
     cookie.sendResponse(cb::mcbp::Status::Success);
 }
 
@@ -254,7 +254,7 @@ static void sasl_list_mech_executor(Cookie& cookie) {
         return;
     }
 
-    if (settings.isExternalAuthServiceEnabled()) {
+    if (Settings::instance().isExternalAuthServiceEnabled()) {
         // If the system is configured to use ns_server for authentication
         // we should only advertise PLAIN as the mechanism (as it can't use
         // SCRAM to connect to LDAP)
@@ -267,16 +267,18 @@ static void sasl_list_mech_executor(Cookie& cookie) {
         return;
     }
 
-    if (connection.isSslEnabled() && settings.has.ssl_sasl_mechanisms) {
-        const auto& mechs = settings.getSslSaslMechanisms();
+    if (connection.isSslEnabled() &&
+        Settings::instance().has.ssl_sasl_mechanisms) {
+        const auto& mechs = Settings::instance().getSslSaslMechanisms();
         cookie.sendResponse(cb::mcbp::Status::Success,
                             {},
                             {},
                             mechs,
                             cb::mcbp::Datatype::Raw,
                             0);
-    } else if (!connection.isSslEnabled() && settings.has.sasl_mechanisms) {
-        const auto& mechs = settings.getSaslMechanisms();
+    } else if (!connection.isSslEnabled() &&
+               Settings::instance().has.sasl_mechanisms) {
+        const auto& mechs = Settings::instance().getSaslMechanisms();
         cookie.sendResponse(cb::mcbp::Status::Success,
                             {},
                             {},
@@ -453,11 +455,11 @@ static void config_reload_executor(Cookie& cookie) {
     // We need to audit that the privilege debug mode changed and
     // in order to do that we need the "connection" object so we can't
     // do this by using the common "changed_listener"-interface.
-    const bool old_priv_debug = settings.isPrivilegeDebug();
+    const bool old_priv_debug = Settings::instance().isPrivilegeDebug();
     reload_config_file();
-    if (settings.isPrivilegeDebug() != old_priv_debug) {
+    if (Settings::instance().isPrivilegeDebug() != old_priv_debug) {
         audit_set_privilege_debug_mode(cookie.getConnection(),
-                                       settings.isPrivilegeDebug());
+                                       Settings::instance().isPrivilegeDebug());
     }
     cookie.sendResponse(cb::mcbp::Status::Success);
 }
@@ -487,7 +489,7 @@ static void get_errmap_executor(Cookie& cookie) {
     auto value = cookie.getRequest(Cookie::PacketContent::Full).getValue();
     auto* req = reinterpret_cast<const cb::mcbp::request::GetErrmapPayload*>(
             value.data());
-    auto const& errormap = settings.getErrorMap(req->getVersion());
+    auto const& errormap = Settings::instance().getErrorMap(req->getVersion());
     if (errormap.empty()) {
         cookie.sendResponse(cb::mcbp::Status::KeyEnoent);
     } else {
@@ -543,7 +545,7 @@ static void rbac_refresh_executor(Cookie& cookie) {
 }
 
 static void auth_provider_executor(Cookie& cookie) {
-    if (!settings.isExternalAuthServiceEnabled()) {
+    if (!Settings::instance().isExternalAuthServiceEnabled()) {
         cookie.setErrorContext(
                 "Support for external authentication service is disabled");
         cookie.sendResponse(cb::mcbp::Status::NotSupported);
@@ -982,7 +984,7 @@ void try_read_mcbp_command(Cookie& cookie) {
         return;
     }
 
-    if (settings.getVerbose() > 1) {
+    if (Settings::instance().getVerbose() > 1) {
         try {
             LOG_TRACE(">{} Read command {}",
                       c.getId(),
@@ -997,13 +999,13 @@ void try_read_mcbp_command(Cookie& cookie) {
 
     // Protect ourself from someone trying to kill us by sending insanely
     // large packets.
-    if (header.getBodylen() > settings.getMaxPacketSize()) {
+    if (header.getBodylen() > Settings::instance().getMaxPacketSize()) {
         LOG_WARNING(
                 "{}: The package size ({}) exceeds the limit ({}) for what "
                 "the system accepts.. Disconnecting client",
                 c.getId(),
                 header.getBodylen(),
-                settings.getMaxPacketSize());
+                Settings::instance().getMaxPacketSize());
         c.setState(StateMachine::State::closing);
         return;
     }