Skip to content

Commit c74d2ee

Browse files
committed
CXX-890 Don't use objects requiring dynamic init/fini for instance::current
1 parent b3061e3 commit c74d2ee

File tree

3 files changed

+31
-19
lines changed

3 files changed

+31
-19
lines changed

src/mongocxx/instance.cpp

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <mongocxx/instance.hpp>
1616

1717
#include <mutex>
18+
#include <type_traits>
1819
#include <utility>
1920

2021
#include <bsoncxx/stdx/make_unique.hpp>
@@ -60,9 +61,10 @@ void user_log_handler(::mongoc_log_level_t mongoc_log_level, const char *log_dom
6061
stdx::string_view{log_domain}, stdx::string_view{message});
6162
}
6263

63-
std::recursive_mutex instance_mutex;
64-
instance *current_instance = nullptr;
65-
std::unique_ptr<instance> global_instance;
64+
std::atomic<instance*> current_instance{nullptr};
65+
static_assert(std::is_standard_layout<decltype(current_instance)>::value, "Must be standard layout");
66+
static_assert(std::is_trivially_constructible<decltype(current_instance)>::value, "Must be trivially constructible");
67+
static_assert(std::is_trivially_destructible<decltype(current_instance)>::value, "Must be trivially destructible");
6668

6769
} // namespace
6870

@@ -94,31 +96,25 @@ instance::instance() : instance(nullptr) {
9496
}
9597

9698
instance::instance(std::unique_ptr<logger> logger) {
97-
std::lock_guard<std::recursive_mutex> lock(instance_mutex);
98-
if (current_instance) {
99+
instance* expected = nullptr;
100+
if (!current_instance.compare_exchange_strong(expected, this))
99101
throw logic_error{error_code::k_instance_already_exists};
100-
}
101102
_impl = stdx::make_unique<impl>(std::move(logger));
102-
current_instance = this;
103103
}
104104

105105
instance::instance(instance &&) noexcept = default;
106106
instance &instance::operator=(instance &&) noexcept = default;
107107

108108
instance::~instance() {
109-
std::lock_guard<std::recursive_mutex> lock(instance_mutex);
110-
if (current_instance != this) std::abort();
111109
_impl.reset();
112-
current_instance = nullptr;
110+
current_instance.store(nullptr);
113111
}
114112

115113
instance &instance::current() {
116-
std::lock_guard<std::recursive_mutex> lock(instance_mutex);
117-
if (!current_instance) {
118-
global_instance.reset(new instance);
119-
current_instance = global_instance.get();
114+
if (!current_instance.load()) {
115+
static instance the_instance;
120116
}
121-
return *current_instance;
117+
return *current_instance.load();
122118
}
123119

124120
MONGOCXX_INLINE_NAMESPACE_END

src/mongocxx/instance.hpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,19 @@ class MONGOCXX_API instance {
5757
~instance();
5858

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

src/mongocxx/test/instance.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,18 @@ TEST_CASE("a user-provided log handler will be used for logging output", "[insta
5757
std::vector<test_log_handler::event> events;
5858
mongocxx::instance driver{stdx::make_unique<test_log_handler>(&events)};
5959

60+
REQUIRE(&mongocxx::instance::current() == &driver);
61+
6062
// The mocking system doesn't play well with varargs functions, so we use a bare
6163
// mongoc_log call here.
6264
mongoc_log(::MONGOC_LOG_LEVEL_ERROR, "foo", "bar");
6365

6466
REQUIRE(events.size() == 1);
6567
REQUIRE(events[0] == std::make_tuple(log_level::k_error, "foo", "bar"));
6668
}
69+
70+
TEST_CASE("after destroying a user constructed instance the instance::current method works") {
71+
mongocxx::instance::current();
72+
}
73+
6774
} // namespace

0 commit comments

Comments
 (0)