diff options
author | ADAM David Alan Martin <adam.martin@10gen.com> | 2018-03-02 16:50:44 -0500 |
---|---|---|
committer | ADAM David Alan Martin <adam.martin@10gen.com> | 2018-03-02 16:51:30 -0500 |
commit | 2084f6702cd026bebe61818564814aac3e2f12a3 (patch) | |
tree | 0d7b43312bc6f110e4a79fcabe03ba74226d2143 | |
parent | ad94e51e0dd40b0d0c38215a36caf75a4be48415 (diff) | |
download | mongo-2084f6702cd026bebe61818564814aac3e2f12a3.tar.gz |
SERVER-33437 Make Decorables not type-erased.
The current Decorable system type-erases the most derived (Decorated)
type when registering Decorations. This is illegal as derived classes
which are no longer standard-layout can have the `Decorable` base
class reordered with respect to the base address of their storage.
This also removes the "with-owner" forms of declaring a decoration,
which caused problems with layout. The decoration handles have
been modified to provide mechanisms to get back to the owner
directly, given a pointer or reference to the decoration.
-rw-r--r-- | src/mongo/db/SConscript | 2 | ||||
-rw-r--r-- | src/mongo/db/auth/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/catalog/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/concurrency/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/repl/SConscript | 5 | ||||
-rw-r--r-- | src/mongo/db/s/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/s/database_sharding_state.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/s/database_sharding_state.h | 5 | ||||
-rw-r--r-- | src/mongo/db/storage/mmap_v1/SConscript | 3 | ||||
-rw-r--r-- | src/mongo/rpc/SConscript | 2 | ||||
-rw-r--r-- | src/mongo/s/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/transport/SConscript | 3 | ||||
-rw-r--r-- | src/mongo/util/SConscript | 10 | ||||
-rw-r--r-- | src/mongo/util/decorable.h | 57 | ||||
-rw-r--r-- | src/mongo/util/decorable_test.cpp | 83 | ||||
-rw-r--r-- | src/mongo/util/decoration_container.cpp | 47 | ||||
-rw-r--r-- | src/mongo/util/decoration_container.h | 42 | ||||
-rw-r--r-- | src/mongo/util/decoration_registry.cpp | 90 | ||||
-rw-r--r-- | src/mongo/util/decoration_registry.h | 108 | ||||
-rw-r--r-- | src/mongo/util/net/SConscript | 1 |
20 files changed, 215 insertions, 258 deletions
diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 7ac4aa70a8b..b9cb84dac26 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -479,7 +479,6 @@ env.Library( '$BUILD_DIR/mongo/transport/transport_layer_common', '$BUILD_DIR/mongo/util/clock_sources', '$BUILD_DIR/mongo/util/concurrency/spin_lock', - '$BUILD_DIR/mongo/util/decorable', '$BUILD_DIR/mongo/util/fail_point', '$BUILD_DIR/mongo/util/net/host', '$BUILD_DIR/mongo/util/periodic_runner', @@ -653,7 +652,6 @@ env.Library( ], LIBDEPS=[ '$BUILD_DIR/mongo/base', - '$BUILD_DIR/mongo/util/decorable', ], ) diff --git a/src/mongo/db/auth/SConscript b/src/mongo/db/auth/SConscript index 5827aa592d1..d36b1854c14 100644 --- a/src/mongo/db/auth/SConscript +++ b/src/mongo/db/auth/SConscript @@ -23,7 +23,6 @@ env.Library( ], LIBDEPS = [ '$BUILD_DIR/mongo/base', - '$BUILD_DIR/mongo/util/decorable', '$BUILD_DIR/mongo/util/net/network', ], ) diff --git a/src/mongo/db/catalog/SConscript b/src/mongo/db/catalog/SConscript index db9f6516cfa..3ba44290c72 100644 --- a/src/mongo/db/catalog/SConscript +++ b/src/mongo/db/catalog/SConscript @@ -219,7 +219,6 @@ env.Library( '$BUILD_DIR/mongo/base', '$BUILD_DIR/mongo/db/namespace_string', '$BUILD_DIR/mongo/db/storage/storage_options', - '$BUILD_DIR/mongo/util/decorable', ], ) diff --git a/src/mongo/db/concurrency/SConscript b/src/mongo/db/concurrency/SConscript index b14ddc7e836..41c5abace77 100644 --- a/src/mongo/db/concurrency/SConscript +++ b/src/mongo/db/concurrency/SConscript @@ -42,7 +42,6 @@ env.Library( LIBDEPS=[ '$BUILD_DIR/mongo/base', '$BUILD_DIR/mongo/util/background_job', - '$BUILD_DIR/mongo/util/decorable', # Temporary crutch since the ssl cleanup is hard coded in background.cpp '$BUILD_DIR/mongo/db/server_parameters', diff --git a/src/mongo/db/repl/SConscript b/src/mongo/db/repl/SConscript index 0f13e84726f..a8df37f3436 100644 --- a/src/mongo/db/repl/SConscript +++ b/src/mongo/db/repl/SConscript @@ -229,7 +229,6 @@ env.Library( LIBDEPS=[ 'optime', '$BUILD_DIR/mongo/db/service_context', - '$BUILD_DIR/mongo/util/decorable', ], ) @@ -343,7 +342,6 @@ env.Library( 'rollback_idl', 'storage_interface', '$BUILD_DIR/mongo/db/service_context', - '$BUILD_DIR/mongo/util/decorable', ], ) @@ -372,7 +370,6 @@ env.Library( '$BUILD_DIR/mongo/base', '$BUILD_DIR/mongo/db/namespace_string', '$BUILD_DIR/mongo/db/service_context', - '$BUILD_DIR/mongo/util/decorable', ], ) @@ -951,7 +948,6 @@ env.Library( '$BUILD_DIR/mongo/db/namespace_string', '$BUILD_DIR/mongo/db/service_context', '$BUILD_DIR/mongo/util/net/network', - '$BUILD_DIR/mongo/util/decorable', ], ) @@ -1325,7 +1321,6 @@ env.Library( '$BUILD_DIR/mongo/db/service_context', '$BUILD_DIR/mongo/unittest/concurrency', '$BUILD_DIR/mongo/unittest/unittest', - '$BUILD_DIR/mongo/util/decorable', ], ) diff --git a/src/mongo/db/s/SConscript b/src/mongo/db/s/SConscript index cda73aa0065..da09c41fdf2 100644 --- a/src/mongo/db/s/SConscript +++ b/src/mongo/db/s/SConscript @@ -16,7 +16,6 @@ env.Library( ], LIBDEPS=[ '$BUILD_DIR/mongo/base', - '$BUILD_DIR/mongo/util/decorable', '$BUILD_DIR/mongo/db/range_arithmetic', '$BUILD_DIR/mongo/s/sharding_routing_table', ], diff --git a/src/mongo/db/s/database_sharding_state.cpp b/src/mongo/db/s/database_sharding_state.cpp index 40a630bc5b4..952fd1c3695 100644 --- a/src/mongo/db/s/database_sharding_state.cpp +++ b/src/mongo/db/s/database_sharding_state.cpp @@ -34,12 +34,12 @@ namespace mongo { const Database::Decoration<DatabaseShardingState> DatabaseShardingState::get = - Database::declareDecorationWithOwner<DatabaseShardingState>(); + Database::declareDecoration<DatabaseShardingState>(); -DatabaseShardingState::DatabaseShardingState(Database* db) : _db(db) {} +DatabaseShardingState::DatabaseShardingState() = default; void DatabaseShardingState::enterCriticalSection(OperationContext* opCtx) { - invariant(opCtx->lockState()->isDbLockedForMode(_db->name(), MODE_X)); + invariant(opCtx->lockState()->isDbLockedForMode(get.owner(this)->name(), MODE_X)); invariant(!_critSecSignal); _critSecSignal = std::make_shared<Notification<void>>(); // TODO (SERVER-33313): call CursorManager::invalidateAll() on all collections in this database @@ -49,7 +49,7 @@ void DatabaseShardingState::enterCriticalSection(OperationContext* opCtx) { void DatabaseShardingState::exitCriticalSection(OperationContext* opCtx, boost::optional<DatabaseVersion> newDbVersion) { - invariant(opCtx->lockState()->isDbLockedForMode(_db->name(), MODE_X)); + invariant(opCtx->lockState()->isDbLockedForMode(get.owner(this)->name(), MODE_X)); invariant(_critSecSignal); _critSecSignal->set(); _critSecSignal.reset(); @@ -62,7 +62,7 @@ std::shared_ptr<Notification<void>> DatabaseShardingState::getCriticalSectionSig void DatabaseShardingState::setDbVersion(OperationContext* opCtx, boost::optional<DatabaseVersion> newDbVersion) { - invariant(opCtx->lockState()->isDbLockedForMode(_db->name(), MODE_X)); + invariant(opCtx->lockState()->isDbLockedForMode(get.owner(this)->name(), MODE_X)); _dbVersion = newDbVersion; } diff --git a/src/mongo/db/s/database_sharding_state.h b/src/mongo/db/s/database_sharding_state.h index 28c0d94df22..50e6e0239eb 100644 --- a/src/mongo/db/s/database_sharding_state.h +++ b/src/mongo/db/s/database_sharding_state.h @@ -45,7 +45,7 @@ class DatabaseShardingState { public: static const Database::Decoration<DatabaseShardingState> get; - DatabaseShardingState(Database* db); + DatabaseShardingState(); ~DatabaseShardingState() = default; /** @@ -84,9 +84,6 @@ public: void checkDbVersion(OperationContext* opCtx) const; private: - // The database to which this sharding state corresponds. - const Database* _db; - // Modifying the state below requires holding the DBLock in X mode; holding the DBLock in any // mode is acceptable for reading it. (Note: accessing this class at all requires holding the // DBLock in some mode, since it requires having a pointer to the Database). diff --git a/src/mongo/db/storage/mmap_v1/SConscript b/src/mongo/db/storage/mmap_v1/SConscript index 8ec5003fd68..eaa8da9a00b 100644 --- a/src/mongo/db/storage/mmap_v1/SConscript +++ b/src/mongo/db/storage/mmap_v1/SConscript @@ -239,7 +239,6 @@ if mmapv1: source=['record_store_v1_simple_test.cpp', ], LIBDEPS=[ - '$BUILD_DIR/mongo/util/decorable', 'record_store_v1_test_help' ] ) @@ -249,7 +248,6 @@ if mmapv1: source=['record_store_v1_capped_test.cpp', ], LIBDEPS=[ - '$BUILD_DIR/mongo/util/decorable', 'record_store_v1_test_help' ] ) @@ -282,7 +280,6 @@ if mmapv1: source=['btree/btree_logic_test.cpp' ], LIBDEPS=[ - '$BUILD_DIR/mongo/util/decorable', 'btree_test_help' ] ) diff --git a/src/mongo/rpc/SConscript b/src/mongo/rpc/SConscript index 98ea0e6d64c..c3b364020a7 100644 --- a/src/mongo/rpc/SConscript +++ b/src/mongo/rpc/SConscript @@ -150,7 +150,6 @@ env.Clone().InjectModule("enterprise").Library( '$BUILD_DIR/mongo/db/logical_time_validator', '$BUILD_DIR/mongo/db/repl/optime', '$BUILD_DIR/mongo/db/signed_logical_time', - '$BUILD_DIR/mongo/util/decorable', ], ) @@ -225,7 +224,6 @@ env.Library( '$BUILD_DIR/mongo/s/is_mongos', '$BUILD_DIR/mongo/transport/transport_layer', "$BUILD_DIR/mongo/util/concurrency/spin_lock", - '$BUILD_DIR/mongo/util/decorable', '$BUILD_DIR/mongo/util/net/network', "$BUILD_DIR/mongo/util/processinfo", ], diff --git a/src/mongo/s/SConscript b/src/mongo/s/SConscript index 83c784ff0be..7339848dac1 100644 --- a/src/mongo/s/SConscript +++ b/src/mongo/s/SConscript @@ -355,7 +355,6 @@ env.Library( LIBDEPS=[ '$BUILD_DIR/mongo/base', '$BUILD_DIR/mongo/client/connection_string', - '$BUILD_DIR/mongo/util/decorable', ] ) diff --git a/src/mongo/transport/SConscript b/src/mongo/transport/SConscript index f5cce983eac..6caf11beb61 100644 --- a/src/mongo/transport/SConscript +++ b/src/mongo/transport/SConscript @@ -13,7 +13,6 @@ env.Library( ], LIBDEPS=[ '$BUILD_DIR/mongo/base', - '$BUILD_DIR/mongo/util/decorable', ], ) @@ -176,7 +175,6 @@ env.CppUnitTest( '$BUILD_DIR/mongo/rpc/command_request', '$BUILD_DIR/mongo/unittest/unittest', '$BUILD_DIR/mongo/util/clock_source_mock', - '$BUILD_DIR/mongo/util/decorable', ], ) @@ -193,7 +191,6 @@ zlibEnv.Library( ], LIBDEPS=[ '$BUILD_DIR/mongo/base', - '$BUILD_DIR/mongo/util/decorable', '$BUILD_DIR/mongo/util/options_parser/options_parser', '$BUILD_DIR/third_party/shim_snappy', '$BUILD_DIR/third_party/shim_zlib', diff --git a/src/mongo/util/SConscript b/src/mongo/util/SConscript index dbac968868e..dc46c7a6fa9 100644 --- a/src/mongo/util/SConscript +++ b/src/mongo/util/SConscript @@ -59,22 +59,12 @@ debuggerEnv.Library( ] ) -env.Library( - target='decorable', - source=[ - 'decoration_container.cpp', - 'decoration_registry.cpp', - ], - LIBDEPS=[] - ) - env.CppUnitTest( target='decorable_test', source=[ 'decorable_test.cpp' ], LIBDEPS=[ - 'decorable', ] ) diff --git a/src/mongo/util/decorable.h b/src/mongo/util/decorable.h index 0545ec2f6ae..2589fac0262 100644 --- a/src/mongo/util/decorable.h +++ b/src/mongo/util/decorable.h @@ -59,7 +59,6 @@ #pragma once -#include "mongo/base/disallow_copying.h" #include "mongo/util/decoration_container.h" #include "mongo/util/decoration_registry.h" @@ -67,7 +66,8 @@ namespace mongo { template <typename D> class Decorable { - MONGO_DISALLOW_COPYING(Decorable); + Decorable(const Decorable&) = delete; + Decorable& operator=(const Decorable&) = delete; public: template <typename T> @@ -76,51 +76,72 @@ public: Decoration() = delete; T& operator()(D& d) const { - return static_cast<Decorable&>(d)._decorations.getDecoration(_raw); + return static_cast<Decorable&>(d)._decorations.getDecoration(this->_raw); } - T& operator()(D* d) const { + T& operator()(D* const d) const { return (*this)(*d); } const T& operator()(const D& d) const { - return static_cast<const Decorable&>(d)._decorations.getDecoration(_raw); + return static_cast<const Decorable&>(d)._decorations.getDecoration(this->_raw); } - const T& operator()(const D* d) const { + const T& operator()(const D* const d) const { return (*this)(*d); } + const D* owner(const T* const t) const { + return static_cast<const D*>(getOwnerImpl(t)); + } + + D* owner(T* const t) const { + return static_cast<D*>(getOwnerImpl(t)); + } + + const D& owner(const T& t) const { + return *owner(&t); + } + + D& owner(T& t) const { + return *owner(&t); + } + private: + const Decorable* getOwnerImpl(const T* const t) const { + return *reinterpret_cast<const Decorable* const*>( + reinterpret_cast<const unsigned char* const>(t) - _raw._raw._index); + } + + Decorable* getOwnerImpl(T* const t) const { + return const_cast<Decorable*>(getOwnerImpl(const_cast<const T*>(t))); + } + friend class Decorable; - explicit Decoration(DecorationContainer::DecorationDescriptorWithType<T> raw) + explicit Decoration( + typename DecorationContainer<D>::template DecorationDescriptorWithType<T> raw) : _raw(std::move(raw)) {} - DecorationContainer::DecorationDescriptorWithType<T> _raw; + typename DecorationContainer<D>::template DecorationDescriptorWithType<T> _raw; }; template <typename T> static Decoration<T> declareDecoration() { - return Decoration<T>(getRegistry()->declareDecoration<T>()); - } - - template <typename T> - static Decoration<T> declareDecorationWithOwner() { - return Decoration<T>(getRegistry()->declareDecorationWithOwner<T, D>()); + return Decoration<T>(getRegistry()->template declareDecoration<T>()); } protected: - Decorable() : _decorations(getRegistry(), this) {} + Decorable() : _decorations(this, getRegistry()) {} ~Decorable() = default; private: - static DecorationRegistry* getRegistry() { - static DecorationRegistry* theRegistry = new DecorationRegistry(); + static DecorationRegistry<D>* getRegistry() { + static DecorationRegistry<D>* theRegistry = new DecorationRegistry<D>(); return theRegistry; } - DecorationContainer _decorations; + DecorationContainer<D> _decorations; }; } // namespace mongo diff --git a/src/mongo/util/decorable_test.cpp b/src/mongo/util/decorable_test.cpp index 356cc9cfb64..8f075a9ec87 100644 --- a/src/mongo/util/decorable_test.cpp +++ b/src/mongo/util/decorable_test.cpp @@ -65,8 +65,9 @@ public: int value; }; +class MyDecorable : public Decorable<MyDecorable> {}; + TEST(DecorableTest, DecorableType) { - class MyDecorable : public Decorable<MyDecorable> {}; const auto dd1 = MyDecorable::declareDecoration<A>(); const auto dd2 = MyDecorable::declareDecoration<A>(); const auto dd3 = MyDecorable::declareDecoration<int>(); @@ -102,16 +103,16 @@ TEST(DecorableTest, DecorableType) { TEST(DecorableTest, SimpleDecoration) { numConstructedAs = 0; numDestructedAs = 0; - DecorationRegistry registry; + DecorationRegistry<MyDecorable> registry; const auto dd1 = registry.declareDecoration<A>(); const auto dd2 = registry.declareDecoration<A>(); const auto dd3 = registry.declareDecoration<int>(); { - DecorationContainer decorable1(®istry, nullptr); + DecorationContainer<MyDecorable> decorable1(nullptr, ®istry); ASSERT_EQ(2, numConstructedAs); ASSERT_EQ(0, numDestructedAs); - DecorationContainer decorable2(®istry, nullptr); + DecorationContainer<MyDecorable> decorable2(nullptr, ®istry); ASSERT_EQ(4, numConstructedAs); ASSERT_EQ(0, numDestructedAs); @@ -138,13 +139,13 @@ TEST(DecorableTest, ThrowingConstructor) { numConstructedAs = 0; numDestructedAs = 0; - DecorationRegistry registry; + DecorationRegistry<MyDecorable> registry; registry.declareDecoration<A>(); registry.declareDecoration<ThrowA>(); registry.declareDecoration<A>(); try { - DecorationContainer d(®istry, nullptr); + DecorationContainer<MyDecorable> d(nullptr, ®istry); } catch (const AssertionException& ex) { ASSERT_EQ(ErrorCodes::Unauthorized, ex.code()); } @@ -153,12 +154,12 @@ TEST(DecorableTest, ThrowingConstructor) { } TEST(DecorableTest, Alignment) { - DecorationRegistry registry; + DecorationRegistry<MyDecorable> registry; const auto firstChar = registry.declareDecoration<char>(); const auto firstInt = registry.declareDecoration<int>(); const auto secondChar = registry.declareDecoration<int>(); const auto secondInt = registry.declareDecoration<int>(); - DecorationContainer d(®istry, nullptr); + DecorationContainer<MyDecorable> d(nullptr, ®istry); ASSERT_EQ(0U, reinterpret_cast<uintptr_t>(&d.getDecoration(firstChar)) % std::alignment_of<char>::value); @@ -173,19 +174,69 @@ TEST(DecorableTest, Alignment) { std::alignment_of<int>::value); } +struct DecoratedOwnerChecker : public Decorable<DecoratedOwnerChecker> { + const char answer[100] = "The answer to life the universe and everything is 42"; +}; -class MyDecorable : public Decorable<MyDecorable> {}; +// Test all 4 variations of the owner back reference: const pointer, non-const pointer, const +// reference, non-const reference. +struct DecorationWithOwner { + DecorationWithOwner() {} -class ClassWithOwnerPtr { -public: - ClassWithOwnerPtr(MyDecorable* decorable) : owner(decorable) {} - MyDecorable* owner; + static const DecoratedOwnerChecker::Decoration<DecorationWithOwner> get; + + std::string getTheAnswer1() const { + // const pointer variant + auto* const owner = get.owner(this); + static_assert(std::is_same<const DecoratedOwnerChecker* const, decltype(owner)>::value, + "Type of fetched owner pointer is incorrect."); + return owner->answer; + } + + std::string getTheAnswer2() { + // non-const pointer variant + DecoratedOwnerChecker* const owner = get.owner(this); + return owner->answer; + } + + std::string getTheAnswer3() const { + // const reference variant + auto& owner = get.owner(*this); + static_assert(std::is_same<const DecoratedOwnerChecker&, decltype(owner)>::value, + "Type of fetched owner reference is incorrect."); + return owner.answer; + } + + std::string getTheAnswer4() { + // Non-const reference variant + DecoratedOwnerChecker& owner = get.owner(*this); + return owner.answer; + } }; +const DecoratedOwnerChecker::Decoration<DecorationWithOwner> DecorationWithOwner::get = + DecoratedOwnerChecker::declareDecoration<DecorationWithOwner>(); + + TEST(DecorableTest, DecorationWithOwner) { - const auto get = MyDecorable::declareDecorationWithOwner<ClassWithOwnerPtr>(); - MyDecorable decorable; - invariant(get(decorable).owner == &decorable); + DecoratedOwnerChecker owner; + const std::string answer = owner.answer; + ASSERT_NE(answer, ""); + + const std::string witness1 = DecorationWithOwner::get(owner).getTheAnswer1(); + ASSERT_EQ(answer, witness1); + + const std::string witness2 = DecorationWithOwner::get(owner).getTheAnswer2(); + ASSERT_EQ(answer, witness2); + + const std::string witness3 = DecorationWithOwner::get(owner).getTheAnswer3(); + ASSERT_EQ(answer, witness3); + + const std::string witness4 = DecorationWithOwner::get(owner).getTheAnswer4(); + ASSERT_EQ(answer, witness4); + + DecorationWithOwner& decoration = DecorationWithOwner::get(owner); + ASSERT_EQ(&owner, &DecorationWithOwner::get.owner(decoration)); } } // namespace diff --git a/src/mongo/util/decoration_container.cpp b/src/mongo/util/decoration_container.cpp deleted file mode 100644 index b78cdd92bac..00000000000 --- a/src/mongo/util/decoration_container.cpp +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Copyright (C) 2015 MongoDB Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see <http://www.gnu.org/licenses/>. - * - * As a special exception, the copyright holders give permission to link the - * code of portions of this program with the OpenSSL library under certain - * conditions as described in each individual source file and distribute - * linked combinations including the program with the OpenSSL library. You - * must comply with the GNU Affero General Public License in all respects for - * all of the code used other than as permitted herein. If you modify file(s) - * with this exception, you may extend this exception to your version of the - * file(s), but you are not obligated to do so. If you do not wish to do so, - * delete this exception statement from your version. If you delete this - * exception statement from all source files in the program, then also delete - * it in the license file. - */ - -#include "mongo/platform/basic.h" - -#include "mongo/util/decoration_container.h" - -#include "mongo/util/decoration_registry.h" - -namespace mongo { - -DecorationContainer::DecorationContainer(const DecorationRegistry* registry, void* owner) - : _registry(registry), - _decorationData(new unsigned char[registry->getDecorationBufferSizeBytes()]) { - _registry->construct(this, owner); -} - -DecorationContainer::~DecorationContainer() { - _registry->destruct(this); -} - -} // namespace mongo diff --git a/src/mongo/util/decoration_container.h b/src/mongo/util/decoration_container.h index e4fb89a4cff..647131bf09f 100644 --- a/src/mongo/util/decoration_container.h +++ b/src/mongo/util/decoration_container.h @@ -31,17 +31,21 @@ #include <cstdint> #include <memory> -#include "mongo/base/disallow_copying.h" - namespace mongo { +template <typename DecoratedType> class DecorationRegistry; +template <typename DecoratedType> +class Decorable; + /** * An container for decorations. */ +template <typename DecoratedType> class DecorationContainer { - MONGO_DISALLOW_COPYING(DecorationContainer); + DecorationContainer(const DecorationContainer&) = delete; + DecorationContainer& operator=(const DecorationContainer&) = delete; public: /** @@ -53,8 +57,9 @@ public: DecorationDescriptor() = default; private: - friend class DecorationContainer; - friend class DecorationRegistry; + friend DecorationContainer; + friend DecorationRegistry<DecoratedType>; + friend Decorable<DecoratedType>; explicit DecorationDescriptor(size_t index) : _index(index) {} @@ -72,8 +77,9 @@ public: DecorationDescriptorWithType() = default; private: - friend class DecorationContainer; - friend class DecorationRegistry; + friend DecorationContainer; + friend DecorationRegistry<DecoratedType>; + friend Decorable<DecoratedType>; explicit DecorationDescriptorWithType(DecorationDescriptor raw) : _raw(std::move(raw)) {} @@ -87,8 +93,24 @@ public: * have any declareDecoration() calls made on it while a DecorationContainer dependent on it * is in scope. */ - explicit DecorationContainer(const DecorationRegistry* registry, void* owner); - ~DecorationContainer(); + explicit DecorationContainer(Decorable<DecoratedType>* const decorated, + const DecorationRegistry<DecoratedType>* const registry) + : _registry(registry), + _decorationData(new unsigned char[registry->getDecorationBufferSizeBytes()]) { + // Because the decorations live in the externally allocated storage buffer at + // `_decorationData`, there needs to be a way to get back from a known location within this + // buffer to the type which owns those decorations. We place a pointer to ourselves, a + // "back link" in the front of this storage buffer, as this is the easiest "well known + // location" to compute. + Decorable<DecoratedType>** const backLink = + reinterpret_cast<Decorable<DecoratedType>**>(_decorationData.get()); + *backLink = decorated; + _registry->construct(this); + } + + ~DecorationContainer() { + _registry->destroy(this); + } /** * Gets the decorated value for the given descriptor. @@ -123,7 +145,7 @@ public: } private: - const DecorationRegistry* const _registry; + const DecorationRegistry<DecoratedType>* const _registry; const std::unique_ptr<unsigned char[]> _decorationData; }; diff --git a/src/mongo/util/decoration_registry.cpp b/src/mongo/util/decoration_registry.cpp deleted file mode 100644 index 8ccc838bbe4..00000000000 --- a/src/mongo/util/decoration_registry.cpp +++ /dev/null @@ -1,90 +0,0 @@ -/** - * Copyright (C) 2015 MongoDB Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see <http://www.gnu.org/licenses/>. - * - * As a special exception, the copyright holders give permission to link the - * code of portions of this program with the OpenSSL library under certain - * conditions as described in each individual source file and distribute - * linked combinations including the program with the OpenSSL library. You - * must comply with the GNU Affero General Public License in all respects for - * all of the code used other than as permitted herein. If you modify file(s) - * with this exception, you may extend this exception to your version of the - * file(s), but you are not obligated to do so. If you do not wish to do so, - * delete this exception statement from your version. If you delete this - * exception statement from all source files in the program, then also delete - * it in the license file. - */ - -#include "mongo/platform/basic.h" - -#include "mongo/util/decoration_registry.h" - -namespace mongo { - -DecorationContainer::DecorationDescriptor DecorationRegistry::declareDecoration( - const size_t sizeBytes, - const size_t alignBytes, - const DecorationConstructorFn constructor, - const DecorationDestructorFn destructor) { - const size_t misalignment = _totalSizeBytes % alignBytes; - if (misalignment) { - _totalSizeBytes += alignBytes - misalignment; - } - DecorationContainer::DecorationDescriptor result(_totalSizeBytes); - _decorationInfo.push_back(DecorationInfo(result, constructor, destructor)); - _totalSizeBytes += sizeBytes; - return result; -} - -void DecorationRegistry::construct(DecorationContainer* container, void* owner) const { - auto iter = _decorationInfo.cbegin(); - try { - for (; iter != _decorationInfo.cend(); ++iter) { - iter->constructor(container->getDecoration(iter->descriptor), owner); - } - } catch (...) { - try { - while (iter != _decorationInfo.cbegin()) { - --iter; - iter->destructor(container->getDecoration(iter->descriptor)); - } - } catch (...) { - std::terminate(); - } - throw; - } -} - -void DecorationRegistry::destruct(DecorationContainer* container) const { - try { - for (DecorationInfoVector::const_reverse_iterator iter = _decorationInfo.rbegin(), - end = _decorationInfo.rend(); - iter != end; - ++iter) { - iter->destructor(container->getDecoration(iter->descriptor)); - } - } catch (...) { - std::terminate(); - } -} - -DecorationRegistry::DecorationInfo::DecorationInfo( - DecorationContainer::DecorationDescriptor inDescriptor, - DecorationConstructorFn inConstructor, - DecorationDestructorFn inDestructor) - : descriptor(std::move(inDescriptor)), - constructor(std::move(inConstructor)), - destructor(std::move(inDestructor)) {} - -} // namespace mongo diff --git a/src/mongo/util/decoration_registry.h b/src/mongo/util/decoration_registry.h index b6a3d879609..cbabc096d8a 100644 --- a/src/mongo/util/decoration_registry.h +++ b/src/mongo/util/decoration_registry.h @@ -28,6 +28,9 @@ #pragma once +#include <algorithm> +#include <functional> +#include <iterator> #include <type_traits> #include <vector> @@ -35,6 +38,7 @@ #include "mongo/base/static_assert.h" #include "mongo/stdx/functional.h" #include "mongo/util/decoration_container.h" +#include "mongo/util/scopeguard.h" namespace mongo { @@ -46,6 +50,7 @@ namespace mongo { * the decorations declared on r1, and a DecorationContainer constructed from r2 has instances * of the decorations declared on r2. */ +template <typename DecoratedType> class DecorationRegistry { MONGO_DISALLOW_COPYING(DecorationRegistry); @@ -59,25 +64,15 @@ public: * NOTE: T's destructor must not throw exceptions. */ template <typename T> - DecorationContainer::DecorationDescriptorWithType<T> declareDecoration() { + auto declareDecoration() { MONGO_STATIC_ASSERT_MSG(std::is_nothrow_destructible<T>::value, "Decorations must be nothrow destructible"); - return DecorationContainer::DecorationDescriptorWithType<T>(std::move(declareDecoration( - sizeof(T), std::alignment_of<T>::value, &constructAt<T>, &destructAt<T>))); + return + typename DecorationContainer<DecoratedType>::template DecorationDescriptorWithType<T>( + std::move(declareDecoration( + sizeof(T), std::alignment_of<T>::value, &constructAt<T>, &destroyAt<T>))); } - template <typename T, typename Owner> - DecorationContainer::DecorationDescriptorWithType<T> declareDecorationWithOwner() { - MONGO_STATIC_ASSERT_MSG(std::is_nothrow_destructible<T>::value, - "Decorations must be nothrow destructible"); - return DecorationContainer::DecorationDescriptorWithType<T>( - std::move(declareDecoration(sizeof(T), - std::alignment_of<T>::value, - &constructAtWithOwner<T, Owner>, - &destructAt<T>))); - } - - size_t getDecorationBufferSizeBytes() const { return _totalSizeBytes; } @@ -88,33 +83,67 @@ public: * * Called by the DecorationContainer constructor. Do not call directly. */ - void construct(DecorationContainer* decorable, void* owner) const; + void construct(DecorationContainer<DecoratedType>* const container) const { + using std::cbegin; + + auto iter = cbegin(_decorationInfo); + + auto cleanupFunction = [&iter, container, this ]() noexcept->void { + using std::crend; + std::for_each(std::make_reverse_iterator(iter), + crend(this->_decorationInfo), + [&](auto&& decoration) { + decoration.destructor( + container->getDecoration(decoration.descriptor)); + }); + }; + + auto cleanup = MakeGuard(std::move(cleanupFunction)); + + using std::cend; + + for (; iter != cend(_decorationInfo); ++iter) { + iter->constructor(container->getDecoration(iter->descriptor)); + } + + cleanup.Dismiss(); + } /** * Destroys the decorations declared in this registry on the given instance of "decorable". * * Called by the DecorationContainer destructor. Do not call directly. */ - void destruct(DecorationContainer* decorable) const; + void destroy(DecorationContainer<DecoratedType>* const container) const noexcept try { + for (auto& decoration : _decorationInfo) { + decoration.destructor(container->getDecoration(decoration.descriptor)); + } + } catch (...) { + std::terminate(); + } private: /** * Function that constructs (initializes) a single instance of a decoration. */ - using DecorationConstructorFn = stdx::function<void(void*, void*)>; + using DecorationConstructorFn = void (*)(void*); /** - * Function that destructs (deinitializes) a single instance of a decoration. + * Function that destroys (deinitializes) a single instance of a decoration. */ - using DecorationDestructorFn = stdx::function<void(void*)>; + using DecorationDestructorFn = void (*)(void*); struct DecorationInfo { DecorationInfo() {} - DecorationInfo(DecorationContainer::DecorationDescriptor descriptor, - DecorationConstructorFn constructor, - DecorationDestructorFn destructor); - - DecorationContainer::DecorationDescriptor descriptor; + DecorationInfo( + typename DecorationContainer<DecoratedType>::DecorationDescriptor inDescriptor, + DecorationConstructorFn inConstructor, + DecorationDestructorFn inDestructor) + : descriptor(std::move(inDescriptor)), + constructor(std::move(inConstructor)), + destructor(std::move(inDestructor)) {} + + typename DecorationContainer<DecoratedType>::DecorationDescriptor descriptor; DecorationConstructorFn constructor; DecorationDestructorFn destructor; }; @@ -122,17 +151,12 @@ private: using DecorationInfoVector = std::vector<DecorationInfo>; template <typename T> - static void constructAt(void* location, void* owner) { + static void constructAt(void* location) { new (location) T(); } - template <typename T, typename Owner> - static void constructAtWithOwner(void* location, void* owner) { - new (location) T(static_cast<Owner*>(owner)); - } - template <typename T> - static void destructAt(void* location) { + static void destroyAt(void* location) { static_cast<T*>(location)->~T(); } @@ -142,13 +166,23 @@ private: * * NOTE: "destructor" must not throw exceptions. */ - DecorationContainer::DecorationDescriptor declareDecoration(size_t sizeBytes, - size_t alignBytes, - DecorationConstructorFn constructor, - DecorationDestructorFn destructor); + typename DecorationContainer<DecoratedType>::DecorationDescriptor declareDecoration( + const size_t sizeBytes, + const size_t alignBytes, + const DecorationConstructorFn constructor, + const DecorationDestructorFn destructor) { + const size_t misalignment = _totalSizeBytes % alignBytes; + if (misalignment) { + _totalSizeBytes += alignBytes - misalignment; + } + typename DecorationContainer<DecoratedType>::DecorationDescriptor result(_totalSizeBytes); + _decorationInfo.push_back(DecorationInfo(result, constructor, destructor)); + _totalSizeBytes += sizeBytes; + return result; + } DecorationInfoVector _decorationInfo; - size_t _totalSizeBytes{0}; + size_t _totalSizeBytes{sizeof(void*)}; }; } // namespace mongo diff --git a/src/mongo/util/net/SConscript b/src/mongo/util/net/SConscript index 6d57e33310a..196f8af47a0 100644 --- a/src/mongo/util/net/SConscript +++ b/src/mongo/util/net/SConscript @@ -37,7 +37,6 @@ env.Library( '$BUILD_DIR/mongo/base', '$BUILD_DIR/mongo/db/auth/auth_rolename', '$BUILD_DIR/mongo/util/concurrency/ticketholder', - '$BUILD_DIR/mongo/util/decorable', 'host', ], LIBDEPS_PRIVATE=[ |