CXX-890 Don't use objects requiring dynamic init/fini for instance::current

Author: acmorrow

examples/mongocxx/CMakeLists.txt

@@ -30,6 +30,7 @@ set(MONGOCXX_EXAMPLES
     exception.cpp
     index.cpp
     inserted_id.cpp
+    instance_management.cpp
     query.cpp
     query_projection.cpp
     remove.cpp

examples/mongocxx/instance_management.cpp

@@ -0,0 +1,99 @@
+// Copyright 2016 MongoDB Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <cstdlib>
+#include <memory>
+
+#include <bsoncxx/stdx/make_unique.hpp>
+#include <bsoncxx/stdx/optional.hpp>
+#include <bsoncxx/stdx/string_view.hpp>
+
+#include <mongocxx/instance.hpp>
+#include <mongocxx/logger.hpp>
+#include <mongocxx/pool.hpp>
+#include <mongocxx/stdx.hpp>
+#include <mongocxx/uri.hpp>
+
+namespace {
+
+// This example demonstrates how one might keep a heap allocated mongocxx::instance and
+// mongocxx::pool object associated in a way that allows dynamic configuration. Most of the examples
+// simply create a stack allocated mongocxx::instance object, which is fine for small examples, but
+// real programs will typically require shared access to a commonly configured instance and
+// connection pool across different translation units and components. By providing a singleton which
+// owns the instance and pool objects, we can defer configuration until we are ready to use MongoDB,
+// and share the instance and pool objects between multiple functions.
+
+class mongo_access {
+public:
+
+    static mongo_access& instance() {
+        static mongo_access instance;
+        return instance;
+    }
+
+    void configure(std::unique_ptr<mongocxx::instance> instance,
+                   std::unique_ptr<mongocxx::pool> pool) {
+        _instance = std::move(instance);
+        _pool = std::move(pool);
+    }
+
+    using connection = mongocxx::pool::entry;
+
+    connection get_connection() {
+        return _pool->acquire();
+    }
+
+    mongocxx::stdx::optional<connection> try_get_connection() {
+        return _pool->try_acquire();
+    }
+
+private:
+    mongo_access() = default;
+
+    std::unique_ptr<mongocxx::instance> _instance = nullptr;
+    std::unique_ptr<mongocxx::pool> _pool = nullptr;
+};
+
+}  // namespace
+
+// The 'configure' and 'do_work' functions use the same mongocxx::instance and mongocxx::pool
+// objects by way of the mongo_access singleton.
+
+void configure(mongocxx::uri uri) {
+    class noop_logger : public mongocxx::logger {
+    public:
+        virtual void operator()(mongocxx::log_level, mongocxx::stdx::string_view,
+                                mongocxx::stdx::string_view message) noexcept {}
+    };
+
+    mongo_access::instance().configure(
+        mongocxx::stdx::make_unique<mongocxx::instance>(mongocxx::stdx::make_unique<noop_logger>()),
+        mongocxx::stdx::make_unique<mongocxx::pool>(std::move(uri))
+    );
+
+}
+
+bool do_work() {
+    auto connection = mongo_access::instance().get_connection();
+    if (!connection)
+        return false;
+    return true;
+}
+
+int main(int argc, char* argv[]) {
+    auto uri = mongocxx::uri{(argc >= 2) ? argv[1] : mongocxx::uri::k_default_uri};
+    configure(std::move(uri));
+    return do_work() ? EXIT_SUCCESS : EXIT_FAILURE;
+}

src/mongocxx/instance.cpp

@@ -14,7 +14,9 @@
 
 #include <mongocxx/instance.hpp>
 
+#include <atomic>
 #include <mutex>
+#include <type_traits>
 #include <utility>
 
 #include <bsoncxx/stdx/make_unique.hpp>
@@ -60,9 +62,14 @@ void user_log_handler(::mongoc_log_level_t mongoc_log_level, const char *log_dom
                                         stdx::string_view{log_domain}, stdx::string_view{message});
 }
 
-std::recursive_mutex instance_mutex;
-instance *current_instance = nullptr;
-std::unique_ptr<instance> global_instance;
+// A region of memory that acts as a sentintel value indicating that an instance object is being
+// destroyed. We only care about the address of this object, never its contents.
+typename std::aligned_storage<sizeof(instance), alignof(instance)>::type sentinel;
+
+std::atomic<instance*> current_instance{nullptr};
+static_assert(std::is_standard_layout<decltype(current_instance)>::value, "Must be standard layout");
+static_assert(std::is_trivially_constructible<decltype(current_instance)>::value, "Must be trivially constructible");
+static_assert(std::is_trivially_destructible<decltype(current_instance)>::value, "Must be trivially destructible");
 
 }  // namespace
 
@@ -94,31 +101,32 @@ instance::instance() : instance(nullptr) {
 }
 
 instance::instance(std::unique_ptr<logger> logger) {
-    std::lock_guard<std::recursive_mutex> lock(instance_mutex);
-    if (current_instance) {
-        throw logic_error{error_code::k_instance_already_exists};
+
+    while (true) {
+        instance* expected = nullptr;
+        if (current_instance.compare_exchange_strong(expected, this))
+            break;
+        if (expected != reinterpret_cast<instance*>(&sentinel))
+            throw logic_error{error_code::k_instance_already_exists};
     }
+
     _impl = stdx::make_unique<impl>(std::move(logger));
-    current_instance = this;
 }
 
 instance::instance(instance &&) noexcept = default;
 instance &instance::operator=(instance &&) noexcept = default;
 
 instance::~instance() {
-    std::lock_guard<std::recursive_mutex> lock(instance_mutex);
-    if (current_instance != this) std::abort();
+    current_instance.store(reinterpret_cast<instance*>(&sentinel));
     _impl.reset();
-    current_instance = nullptr;
+    current_instance.store(nullptr);
 }
 
 instance &instance::current() {
-    std::lock_guard<std::recursive_mutex> lock(instance_mutex);
-    if (!current_instance) {
-        global_instance.reset(new instance);
-        current_instance = global_instance.get();
+    if (!current_instance.load()) {
+         static instance the_instance;
     }
-    return *current_instance;
+    return *current_instance.load();
 }
 
 MONGOCXX_INLINE_NAMESPACE_END

src/mongocxx/instance.hpp

@@ -57,10 +57,19 @@ class MONGOCXX_API instance {
     ~instance();
 
     ///
-    /// Returns the current unique instance of the driver. If an
-    /// instance was explicitly created, that will be returned. If no
-    /// instance has yet been created, a default instance will be
-    /// constructed and returned.
+    /// Returns the current unique instance of the driver. If an instance was explicitly created,
+    /// that will be returned. If no instance has yet been created, a default instance will be
+    /// constructed and returned. If a default instance is constructed, its destruction will be
+    /// sequenced according to the rules for the destruction of static local variables at program
+    /// exit (see http://en.cppreference.com/w/cpp/utility/program/exit).
+    ///
+    /// Note that, if you need to configure the instance in any way (e.g. with a logger), you cannot
+    /// use this method to cause the instance to be constructed. You must explicitly create an
+    /// properly configured instance object. You can, however, use this method to obtain that
+    /// configured instance object.
+    ///
+    /// @note This method is intended primarily for test authors, where managing the lifetime of the
+    /// instance w.r.t. the test framework can be problematic.
     ///
     static instance& current();
 

src/mongocxx/pool.hpp

@@ -69,7 +69,7 @@ class MONGOCXX_API pool {
     /// @note The lifetime of any entry object must be a subset of the pool object
     ///  from which it was acquired.
     ///
-    using entry = std::unique_ptr<client, std::function<void(client*)>>;
+    using entry = std::unique_ptr<client, std::function<void MONGOCXX_CALL (client*)>>;
 
     ///
     /// Acquires a client from the pool. The calling thread will block until a connection is

src/mongocxx/test/instance.cpp

@@ -57,11 +57,18 @@ TEST_CASE("a user-provided log handler will be used for logging output", "[insta
     std::vector<test_log_handler::event> events;
     mongocxx::instance driver{stdx::make_unique<test_log_handler>(&events)};
 
+    REQUIRE(&mongocxx::instance::current() == &driver);
+
     // The mocking system doesn't play well with varargs functions, so we use a bare
     // mongoc_log call here.
     mongoc_log(::MONGOC_LOG_LEVEL_ERROR, "foo", "bar");
 
     REQUIRE(events.size() == 1);
     REQUIRE(events[0] == std::make_tuple(log_level::k_error, "foo", "bar"));
 }
+
+TEST_CASE("after destroying a user constructed instance the instance::current method works") {
+    mongocxx::instance::current();
+}
+
 }  // namespace