From 260a41b5d32757cc944d3154aabdad5d1443277f Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Thu, 10 Sep 2026 21:15:41 -0400 Subject: [PATCH 1/5] initialize: move the last throwing statement before the commit #95 made initialize() transactional, but `transaction.commit()` was called one statement too early: the `++tr << "Installing\n"` that follows it goes through the `output` policy, which is user code and can throw. When it does, the transaction destructor sees a committed transaction and restores nothing, so the policies keep the v-table pointers they read out of `new_dispatch_data` - the local vector unwinding is about to free. The registry still holds the previous dispatch data (the swap never ran), so the next dispatch through the policy state reads freed memory: the exact use-after-free #95 fixed, reached by a different route. ASan, gcc 13, an `output` policy whose stream throws on "Installing" plus initialize(trace(true)): ERROR: AddressSanitizer: heap-use-after-free READ of size 8 ... in resolve_uni<...> freed by ... write_global_data() Move the trace write above commit(), where a throw rolls the policies back, and extract the writes that follow into `commit_global_data()`, declared `noexcept` so that a throwing statement added there terminates loudly instead of silently reopening this. The test grew a case for that path, and two of its existing assertions were vacuous: the failing initialize saw exactly the input the preceding successful one saw, so `fast_perfect_hash` - which re-seeds a fixed PRNG - recomputed identical factors, and `next` resolved to the same overrider. Both held whether or not anything was rolled back; reintroducing the dangling-`next` half of #81 left the suite reporting "No errors detected". Perturb the input between the two calls instead, with two function-local static registrars: an extra class changes the hash factors, the control table and the v-table pointers, and an extra inheritance edge changes what `next` would be set to. The hash assertion also compares the whole policy state now - factors and control table - rather than `hash_range()` alone. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RQG6CbE4o2agseE7bDVzHS --- include/boost/openmethod/initialize.hpp | 30 +++- test/test_initialize_transaction.cpp | 173 ++++++++++++++++++++++-- 2 files changed, 184 insertions(+), 19 deletions(-) diff --git a/include/boost/openmethod/initialize.hpp b/include/boost/openmethod/initialize.hpp index 7f4c4e8c..95d675a1 100644 --- a/include/boost/openmethod/initialize.hpp +++ b/include/boost/openmethod/initialize.hpp @@ -727,6 +727,8 @@ struct registry::compiler : detail::generic_compiler { std::vector::const_iterator group, const bitvec& candidates, bool concrete); void write_global_data(); + void commit_global_data( + std::vector& new_dispatch_data) noexcept; void print(const method_report& report) const; void print_slots(); static void select_dominant_overriders( @@ -1766,10 +1768,10 @@ void registry::compiler::write_global_data() { // v-table pointer is staged in its class_, where the policies read it. // Only then are the shared locations patched - the method_infos' slots // and strides, the overriders' `next`, the class_infos' static_vptr - and - // the dispatch data swapped in; none of that can throw. If a policy - // throws, the registry still holds the previous dispatch state, complete - // and consistent, rather than pointers into a vector that unwinding has - // just freed. + // the dispatch data swapped in, by commit_global_data(), which is + // `noexcept`. If a policy throws, the registry still holds the previous + // dispatch state, complete and consistent, rather than pointers into a + // vector that unwinding has just freed. auto dispatch_data_size = std::accumulate( methods.begin(), methods.end(), std::size_t(0), @@ -1880,12 +1882,26 @@ void registry::compiler::write_global_data() { detail::registry_state_transaction transaction; detail::initialize_policies::fn(*this, options); - transaction.commit(); - - // Commit. Nothing from here on can throw. + // Last statement that can throw: the trace goes through the `output` + // policy, which is user-supplied. After the commit it would be the very + // bug this arrangement exists to prevent - the policies keeping the + // v-table pointers they just read from `new_dispatch_data`, which + // unwinding frees. ++tr << "Installing\n"; + transaction.commit(); + commit_global_data(new_dispatch_data); +} + +// The commit point. Called once every step that can fail has succeeded, and +// `noexcept` so that a throwing statement added here terminates loudly +// instead of leaving the policies pointing into `new_dispatch_data`, which +// the caller destroys on the way out. +template +template +void registry::compiler::commit_global_data( + std::vector& new_dispatch_data) noexcept { for (auto& m : methods) { auto first_info = m.infos[0]; diff --git a/test/test_initialize_transaction.cpp b/test/test_initialize_transaction.cpp index 551618df..900038a1 100644 --- a/test/test_initialize_transaction.cpp +++ b/test/test_initialize_transaction.cpp @@ -3,11 +3,13 @@ // See accompanying file LICENSE_1_0.txt // or copy at http://www.boost.org/LICENSE_1_0.txt) -// initialize() is transactional: if a policy's initialize throws, the -// registry keeps the dispatch state it had before the call - the static -// v-table pointers, the `next` pointers, the dispatch data and every policy's -// state - instead of pointers into a vector that unwinding has freed. It is -// marked as not initialized, though, until a call succeeds. +// initialize() is transactional: if anything between staging the new dispatch +// data and the commit point throws - a policy's initialize, or the trace +// write that announces the installation, which goes through the user-supplied +// `output` policy - the registry keeps the dispatch state it had before the +// call: the static v-table pointers, the `next` pointers, the dispatch data +// and every policy's state, instead of pointers into a vector that unwinding +// has freed. It is marked as not initialized, though, until a call succeeds. #include #include @@ -19,8 +21,10 @@ #include "test_util.hpp" +#include #include #include +#include #include using boost::mp11::mp_list; @@ -85,8 +89,10 @@ struct Animal { virtual ~Animal() = default; }; -struct Dog : Animal {}; +struct Carnivore : Animal {}; +struct Dog : Carnivore {}; struct Cat : Animal {}; +struct Bird : Animal {}; struct BOOST_OPENMETHOD_ID(poke); @@ -104,18 +110,24 @@ auto poke_dog(Dog& dog) -> std::string { return poke::template next>(dog) + " bark"; } +template +auto poke_carnivore(Carnivore& carnivore) -> std::string { + return poke::template next>(carnivore) + + " growl"; +} + template struct snapshot { using vptr_state = typename Registry::template policy::state; using type_hash = typename Registry::template policy; + using hash_state = typename type_hash::state; snapshot() : dispatch_data(Registry::state().dispatch_data.data()), dog_vptr(Registry::template static_vptr), cat_vptr(Registry::template static_vptr), next(poke::template next>), - hash_range(type_hash::hash_range()), policies(Registry::state().policies) { } @@ -123,11 +135,17 @@ struct snapshot { return (detail::get(policies).vptrs); } + // Everything `fast_perfect_hash::initialize` writes: the factors and the + // control table. `hash_range()` alone would miss a rollback that restored + // the range but not the factors. + auto hash() -> decltype(auto) { + return (detail::get(policies)); + } + const detail::word* dispatch_data; vptr_type dog_vptr; vptr_type cat_vptr; decltype(poke::template next>) next; - std::pair hash_range; decltype(Registry::state().policies) policies; }; @@ -138,11 +156,17 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( typename Registry::registry_type>; using vptr_state = typename snapshot::vptr_state; - BOOST_OPENMETHOD_REGISTER(use_classes); + // Dog is registered here with Animal as its direct base, although it + // really derives from Carnivore. The missing edge is added between the two + // initializes, below. + BOOST_OPENMETHOD_REGISTER(use_classes); + BOOST_OPENMETHOD_REGISTER(use_classes); BOOST_OPENMETHOD_REGISTER( typename poke::template override>); BOOST_OPENMETHOD_REGISTER( typename poke::template override>); + BOOST_OPENMETHOD_REGISTER( + typename poke::template override>); Dog dog; Cat cat; @@ -156,6 +180,21 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( snapshot before; + // Perturb the input of the call that is about to fail. Without this it + // would see exactly what the successful call saw, and recompute + // bit-identical values for everything compared below - `fast_perfect_hash` + // re-seeds a fixed PRNG over the same class set, and `next` + // resolves to the same overrider - so the assertions would hold whether or + // not the transaction rolled anything back. These registrars are + // function-local statics: they register on first pass through the + // declaration, here, not before main. `Bird` changes the class set that + // the hash factors, the control table and the v-table pointers are + // computed from; the Carnivore edge inserts `poke_carnivore` between + // `poke_dog` and `poke_animal`, changing what `next` resolves + // to. The final initialize below observes both. + BOOST_OPENMETHOD_REGISTER(use_classes); + BOOST_OPENMETHOD_REGISTER(use_classes); + explosive::armed = true; BOOST_CHECK_THROW(initialize(), std::runtime_error); explosive::armed = false; @@ -171,8 +210,15 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( // rejects under /WX (-Wmicrosoft-cast). BOOST_TEST( (poke::template next> == before.next)); - BOOST_TEST( - (snapshot::type_hash::hash_range() == before.hash_range)); + // Parenthesized for the same reason: Boost.Test cannot print hash factors + // or vectors of type ids. + auto& hash = + detail::get::hash_state>(st.policies); + BOOST_TEST((hash.fn.mult == before.hash().fn.mult)); + BOOST_TEST((hash.fn.shift == before.hash().fn.shift)); + BOOST_TEST((hash.fn.min_value == before.hash().fn.min_value)); + BOOST_TEST((hash.fn.max_value == before.hash().fn.max_value)); + BOOST_TEST((hash.control == before.hash().control)); BOOST_TEST((detail::get(st.policies).vptrs == before.vptrs())); // ...including the state of the policy that threw, after writing to it. BOOST_TEST(Registry::template state().generation == 1); @@ -180,10 +226,17 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( // ...but dispatch is refused until an initialize() succeeds. BOOST_CHECK_THROW(poke::fn(dog), not_initialized); + // A successful call installs what the failed one would have: the + // perturbation is visible in the result, which confirms that the + // assertions above compared values that really do differ between the two + // calls. initialize(); BOOST_TEST(st.initialized); - BOOST_TEST(poke::fn(dog) == "silence bark"); + BOOST_TEST(poke::fn(dog) == "silence growl bark"); BOOST_TEST(poke::fn(cat) == "silence"); + BOOST_TEST( + (poke::template next> != before.next)); + BOOST_TEST((hash.control != before.hash().control)); BOOST_TEST(Registry::template state().generation == 3); } @@ -222,3 +275,99 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( BOOST_TEST(st.initialized); BOOST_TEST(poke::fn(dog) == "silence bark"); } + +// The other way an initialize can fail after the policies have run: the trace +// goes through the `output` policy, which is user code, so the statement that +// announces the installation can throw. It sits before the commit, and must +// stay there - after it, the policies would keep the v-table pointers they +// just read out of the staging vector, which unwinding frees. + +// Discards what it is given, and throws once, on the message named in `trap`. +struct trapping_stream { + static inline const char* trap = nullptr; + + void write(const char* str) { + if (trap != nullptr && std::strstr(str, trap) != nullptr) { + trap = nullptr; + throw std::runtime_error("output"); + } + } + + auto is_on() const -> bool { + return true; + } +}; + +inline auto operator<<(trapping_stream& os, const char* str) + -> trapping_stream& { + os.write(str); + return os; +} + +inline auto operator<<(trapping_stream& os, const std::string_view&) + -> trapping_stream& { + return os; +} + +inline auto operator<<(trapping_stream& os, const void*) -> trapping_stream& { + return os; +} + +inline auto operator<<(trapping_stream& os, void (*)()) -> trapping_stream& { + return os; +} + +inline auto operator<<(trapping_stream& os, std::size_t) -> trapping_stream& { + return os; +} + +struct trapping_output : policies::output { + template + struct fn { + struct state { + trapping_stream os; + }; + + static auto& stream() { + return Registry::template state().os; + } + }; +}; + +template +struct tracing_registry : + test_registry_::template with< + policies::runtime_checks, policies::throw_error_handler, + trapping_output> {}; + +BOOST_AUTO_TEST_CASE(a_throwing_trace_does_not_commit) { + using Registry = tracing_registry<__COUNTER__>; + using vptr_state = typename snapshot::vptr_state; + + BOOST_OPENMETHOD_REGISTER(use_classes); + BOOST_OPENMETHOD_REGISTER(poke::override>); + BOOST_OPENMETHOD_REGISTER(poke::override>); + + Dog dog; + auto& st = Registry::state(); + + initialize(); + BOOST_TEST(poke::fn(dog) == "silence bark"); + + snapshot before; + + trapping_stream::trap = "Installing"; + BOOST_CHECK_THROW(initialize(trace(true)), std::runtime_error); + BOOST_TEST(trapping_stream::trap == nullptr); // it did throw there + + BOOST_TEST(!st.initialized); + BOOST_TEST(st.dispatch_data.data() == before.dispatch_data); + BOOST_TEST(Registry::template static_vptr == before.dog_vptr); + // The one that matters: had the policies been committed, these would be + // pointers into the staging vector, which no longer exists. + BOOST_TEST((detail::get(st.policies).vptrs == before.vptrs())); + + initialize(); + BOOST_TEST(st.initialized); + BOOST_TEST(poke::fn(dog) == "silence bark"); +} From 1cb2b0c714c4e6132cdcb855889b0e3038471cea Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Fri, 11 Sep 2026 20:08:54 -0400 Subject: [PATCH 2/5] doc: say what a call after a failed initialize() does The transaction section says the registry is marked as not initialized and that initialize() must be called again, but not what happens if a method is called before it is. Only `runtime_checks` diagnoses that; otherwise the call dispatches through the previous tables, which is harmless for a registry whose classes are all still loaded and a use-after-free for one whose overriders came out of a library that has since been dlclose'd - the case the section is written for. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JQa4fuiwcfsheZYTCyfPPr --- doc/modules/ROOT/pages/registries_and_policies.adoc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/modules/ROOT/pages/registries_and_policies.adoc b/doc/modules/ROOT/pages/registries_and_policies.adoc index db0f9ed0..a691c415 100644 --- a/doc/modules/ROOT/pages/registries_and_policies.adoc +++ b/doc/modules/ROOT/pages/registries_and_policies.adoc @@ -160,7 +160,10 @@ say - every policy gets its previous state back, and nothing else in the registry is modified: the v-table pointers, `next` pointers and dispatch tables from the previous call all stay in place. The registry is marked as not initialized, though, since that state no longer reflects the registrations, -and cpp:initialize[] must be called again before calling a method. +and cpp:initialize[] must be called again before calling a method. Only a +registry with the cpp:runtime_checks[] policy diagnoses a call made in the +meantime; without it, the call dispatches through the previous tables, which - +after a `dlclose` - may point into unloaded code. A registry can also be created by copying an existing registry's policies, using the cpp:with[] and cpp:without[] nested templates. For example, From 2dc43953888c11cb2f994e46bba6172e766366f5 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Sat, 12 Sep 2026 09:35:28 -0400 Subject: [PATCH 3/5] initialize: save the state of the policies it initializes, and only those #95 made initialize() transactional by copying the registry's whole `policies` tuple and putting it back if anything throws. The comment justified the width - "restoring an untouched state is harmless, and simpler than picking" - but it is not harmless, in two ways. A state that no `initialize` writes to is not derived data the call is about to replace; it is configuration the caller owns. The error handler is the case that bites: it is *called* from inside the transaction window by design, since fast_perfect_hash reports a search failure through it. A handler that disarms itself - installs another handler with set() before throwing, so the failure is reported once - had that undone on the way out. Reported here as a change to the handler surviving or not: before: handler still installed after the failed initialize = 1 after: handler still installed after the failed initialize = 0 And copying the tuple made copy-constructibility a hard requirement of every policy state in the registry, including states of policies that have no `initialize` at all. That rules out a state holding a std::mutex, std::unique_ptr, std::atomic or std::ostringstream - and an ostringstream is exactly what an `output` policy written to the documented state pattern holds. It compiled before #95 and stopped compiling after, with 29 lines of deleted-copy-constructor diagnostics on gcc 13 (207 on clang 18) naming the whole policy list 21 times. Filter the saved states to the policies whose `initialize` will actually run, using the same has_initialize test initialize_policy makes, so the two cannot disagree about which policies run. The saved tuple is now a subset of the registry's, so it is filled and restored element-wise. A static_assert spells out the requirement that remains, in the manner of the one preamble.hpp already carries for duplicate state types. One correction to how this was reported to me: it does *not* remove the copy of the control vector and the vptr vector from the success path. Those two policies define `initialize`, so their states must still be saved - that is what makes a rollback possible. What is no longer copied is the states of the policies that do not initialize: in default_registry, the error handler's std::function and the output policy's state. Not addressed: the restore is still a move-assignment in a `noexcept` destructor, so a policy state whose move-assignment throws still terminates. Stock registries are nothrow, so nothing in the suite can show it. The test covers both halves. It fails to compile without this change, because its configuration policy's state is deliberately move-only; with the state made copyable to isolate the other half, it fails 1 != 2 on the rolled-back generation counter. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JQa4fuiwcfsheZYTCyfPPr --- include/boost/openmethod/initialize.hpp | 94 +++++++++++--- test/test_initialize_policy_state_scope.cpp | 132 ++++++++++++++++++++ 2 files changed, 211 insertions(+), 15 deletions(-) create mode 100644 test/test_initialize_policy_state_scope.cpp diff --git a/include/boost/openmethod/initialize.hpp b/include/boost/openmethod/initialize.hpp index 95d675a1..fd565f34 100644 --- a/include/boost/openmethod/initialize.hpp +++ b/include/boost/openmethod/initialize.hpp @@ -124,27 +124,83 @@ struct initialize_policies> { } }; +// Selects the policies whose state the transaction has to save: those that +// have a `state`, *and* whose `initialize` will actually be called for this +// Context and Options - the same test initialize_policy makes, so the two +// cannot disagree about which policies run. +template +struct policy_state_is_volatile_q { + template + using fn = mp11::mp_bool< + has_policy_state::value && + has_initialize>; +}; + // Saves the policies' states on construction, and puts them back on // destruction unless commit() was called - so a policy's `initialize` that // throws, after itself or another policy has already written to shared // state, leaves the registry as it was. The registry's mutable state is one -// variable, registry_state::st; its `policies` tuple is copied -// whole, states of policies without an `initialize` included, since -// restoring an untouched state is harmless, and simpler than picking. The -// other members need no saving: initialize() only reads the class and -// method lists, and write_global_data() replaces dispatch_data at commit -// time only, after which nothing can throw - on rollback it still holds the -// previous tables, which the classes' static_vptrs point into. (That is -// also why a copy could not stand in for it: it would be another buffer.) -template +// variable, registry_state::st. Its other members need no saving: +// initialize() only reads the class and method lists, and write_global_data() +// replaces dispatch_data at commit time only, after which nothing can throw - +// on rollback it still holds the previous tables, which the classes' +// static_vptrs point into. (That is also why a copy could not stand in for +// it: it would be another buffer.) +// +// Only the states of the policies that are about to be initialized are saved. +// Copying the `policies` tuple whole is simpler, but it is not harmless: a +// state that no `initialize` touches is not derived data this call is about to +// replace, it is configuration the caller owns. The error handler is the case +// that bites - it is *called* from inside the window, by design +// (fast_perfect_hash reports a search failure through it), so a handler that +// disarms itself with set() before throwing would have that undone on the way +// out. Saving wide also forces every policy state in the registry to be +// copyable, including those of policies that have no `initialize` at all - +// which rules out the std::ostringstream an `output` policy written to the +// documented state pattern naturally holds - and copies each of them, vectors +// and all, on every successful initialize(). +template class registry_state_transaction { + using policy_fns = mp11::mp_transform_q< + policy_fn_q, typename Registry::policy_list>; + using saved_states = mp11::mp_transform< + policy_state_t, + mp11::mp_filter_q< + policy_state_is_volatile_q, policy_fns>>; + using saved_type = mp11::mp_apply; + + static_assert( + mp11::mp_all_of::value, + "the `state` of a policy that defines `initialize` must be copyable: " + "initialize() saves it, and puts it back if a policy throws"); + + // Element-wise: `saved` holds a subset of the registry's tuple. + template + struct each; + + template + struct each> { + static void save(detail::tuple& to) { + (..., + (detail::get(to) = + detail::get(Registry::state().policies))); + } + + static void restore(detail::tuple& from) { + (..., + (detail::get(Registry::state().policies) = + std::move(detail::get(from)))); + } + }; + public: - registry_state_transaction() : saved(Registry::state().policies) { + registry_state_transaction() { + each::save(saved); } ~registry_state_transaction() { if (!committed) { - Registry::state().policies = std::move(saved); + each::restore(saved); } } @@ -157,7 +213,7 @@ class registry_state_transaction { } private: - typename registry_state_type::policies_type saved; + saved_type saved; bool committed = false; }; @@ -1880,7 +1936,9 @@ void registry::compiler::write_global_data() { ++tr << rflush(4, dispatch_data_size) << " " << gv_iter << " end\n"; - detail::registry_state_transaction transaction; + detail::registry_state_transaction< + registry, compiler, std::tuple> + transaction; detail::initialize_policies::fn(*this, options); // Last statement that can throw: the trace goes through the `output` @@ -2238,8 +2296,14 @@ void registry::compiler::print_slots() { //! call: no static v-table pointer, `next` pointer, dispatch table or policy //! state is modified. The registry is nonetheless marked as not initialized, //! since that state does not reflect the current registrations; `initialize` -//! must be called again, successfully, before any method is called. Policy -//! states are restored from a copy, so a policy's `state` must be copyable. +//! must be called again, successfully, before any method is called. +//! +//! Only the policies that define `initialize` have their `state` saved and +//! restored, so only those states have to be copyable. A state that no +//! `initialize` writes to is configuration, not derived data, and is left +//! alone: an @ref error_handler policy is *called* during `initialize`, and a +//! handler that changes the configuration - installing a different handler +//! with `set`, say - keeps that change whether the call succeeds or throws. //! //! @par Example //! diff --git a/test/test_initialize_policy_state_scope.cpp b/test/test_initialize_policy_state_scope.cpp new file mode 100644 index 00000000..155bba89 --- /dev/null +++ b/test/test_initialize_policy_state_scope.cpp @@ -0,0 +1,132 @@ +// Copyright (c) 2017-2026 Jean-Louis Leroy +// Distributed under the Boost Software License, Version 1.0. +// See accompanying file LICENSE_1_0.txt +// or copy at http://www.boost.org/LICENSE_1_0.txt) + +// initialize() saves and restores the state of the policies it initializes, +// and only those. A state that no `initialize` writes to is configuration the +// caller owns, not derived data the call is about to replace, so a failed +// initialize() must leave it alone - the error handler is the case that +// matters, since it is called from inside the transaction window by design. +// +// Two consequences, both checked here: such a state survives a rollback, and +// it does not have to be copyable - which it would if the transaction copied +// the whole policy tuple, ruling out the std::ostringstream an `output` policy +// written to the documented state pattern naturally holds. + +#include +#include + +#define BOOST_TEST_MODULE initialize_policy_state_scope +#include + +#include "test_util.hpp" + +#include +#include +#include +#include +#include + +using namespace boost::openmethod; + +// Configuration: it has a `state`, and deliberately no `initialize`. The state +// is move-only, like the std::ostringstream it carries - the transaction must +// not require it to be copyable. +struct config_policy { + using category = config_policy; + + template + struct fn { + struct state { + state() = default; + state(const state&) = delete; + auto operator=(const state&) -> state& = delete; + + int generation = 0; + std::ostringstream os; + }; + }; +}; + +// Writes to its own state and to the configuration policy's - the way an error +// handler called from inside the window changes the handler it installs - then +// throws on demand. +struct explosive_policy { + using category = explosive_policy; + + template + struct fn { + struct state { + int generation = 0; + }; + + inline static bool armed = false; + + template + static void initialize(const Context&, const std::tuple&) { + ++Registry::template state().generation; + ++Registry::template state().generation; + + if (armed) { + throw std::runtime_error("boom"); + } + } + }; +}; + +template +struct test_registry : + test_registry_::template with {}; + +using test_reg = test_registry<__COUNTER__>; +using config_state = config_policy::fn::state; +using explosive = explosive_policy::fn; + +// The point of the exercise: a policy that has no `initialize` may hold a +// state the transaction could not copy even if it wanted to. If this ever +// becomes copyable the test below stops proving anything. +static_assert(!std::is_copy_constructible_v); +static_assert(!std::is_copy_assignable_v); + +struct Animal { + virtual ~Animal() = default; +}; + +struct Dog : Animal {}; + +struct BOOST_OPENMETHOD_ID(poke); + +using poke = method< + BOOST_OPENMETHOD_ID(poke), auto(virtual_)->std::string, test_reg>; + +auto poke_animal(Animal&) -> std::string { + return "silence"; +} + +BOOST_AUTO_TEST_CASE(config_state_survives_a_failed_initialize) { + BOOST_OPENMETHOD_REGISTER(use_classes); + BOOST_OPENMETHOD_REGISTER(poke::override); + + initialize(); + BOOST_TEST(test_reg::state().generation == 1); + BOOST_TEST(test_reg::state().generation == 1); + + explosive::armed = true; + BOOST_CHECK_THROW(initialize(), std::runtime_error); + explosive::armed = false; + + // The initializing policy's own state is derived data: rolled back, so the + // second run's increment is undone. + BOOST_TEST(test_reg::state().generation == 1); + + // The configuration policy's is not: the write made inside the window + // stands, exactly as an error handler that disarms itself before throwing + // would expect. + BOOST_TEST(test_reg::state().generation == 2); + + // And the registry still works. + initialize(); + Dog dog; + BOOST_TEST(poke::fn(dog) == "silence"); +} From 5ca43ebbe68329cff060e205205ea77189dfe805 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Sat, 12 Sep 2026 09:56:34 -0400 Subject: [PATCH 4/5] initialize: report before installing, and refuse a state that can throw on restore Two more ways the transaction could be escaped, both found by the same review that prompted the previous commits. print(report) and print_slots() ran after write_global_data() had committed. Both can throw - the trace goes through the user-supplied `output` policy, and print_slots() builds an unordered_map and two vectors to partition the class graph - and a throw there landed past the commit: the new tables installed, the policies holding the new v-table pointers, and initialize() never reaching `st.initialized = true`. Nothing is corrupted, but it is a fourth outcome the exception-safety contract does not describe, and under runtime_checks the next call aborts with `not_initialized` even though the tables are perfectly good. It survived the previous commit, which moved only the "Installing" write that lives inside write_global_data(). Nothing printed there needs the new tables: the report is gathered during compile(), and the slots and lattices are compiler-local. So report first, install second, and a throw is once again just a failure the transaction rolls back. The last line of a successful trace is now "Installing", which is emitted immediately before the commit. The other is the restore itself. It runs from the transaction's destructor, while an exception is in flight, and a destructor is noexcept by default: a policy state whose move-assignment can throw would terminate the program, destroying the error the transaction exists to let through. That is reachable through the public API - a state holding a std::map with a stateful, non-always-equal allocator degrades to an element-wise move that allocates, and `vptr_map` lets a caller supply exactly that map. Every stock registry is nothrow move-assignable, so no test could ever have shown it. Refuse such a state at compile time, next to the copyability assertion, where the message can say why. Tests: a_throwing_report_does_not_commit trips the trace on "Used slots", which print_slots() writes, and checks the previous dispatch state is intact - it fails all three of those assertions without the reordering. The compile-fail test covers the assertion. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JQa4fuiwcfsheZYTCyfPPr --- include/boost/openmethod/initialize.hpp | 30 ++++++++- ...ompile_fail_policy_state_throwing_move.cpp | 62 +++++++++++++++++++ test/test_initialize_transaction.cpp | 37 +++++++++++ 3 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 test/compile_fail_policy_state_throwing_move.cpp diff --git a/include/boost/openmethod/initialize.hpp b/include/boost/openmethod/initialize.hpp index fd565f34..977da500 100644 --- a/include/boost/openmethod/initialize.hpp +++ b/include/boost/openmethod/initialize.hpp @@ -174,6 +174,21 @@ class registry_state_transaction { "the `state` of a policy that defines `initialize` must be copyable: " "initialize() saves it, and puts it back if a policy throws"); + // The restore runs from the destructor, while an exception is in flight, + // and a destructor is noexcept by default: a move-assignment that threw + // there would call std::terminate, destroying the very error the + // transaction exists to let through. It is not a theoretical shape - a + // state holding a std::map with a stateful, non-always-equal allocator + // degrades to an element-wise move that allocates, and `vptr_map` + // lets a caller supply exactly that. Refuse it here, where the message can + // say why, rather than terminate at run time in the one configuration no + // test covers. + static_assert( + mp11::mp_all_of::value, + "the `state` of a policy that defines `initialize` must be nothrow " + "move-assignable: initialize() puts it back from a destructor, while " + "an exception is in flight, where throwing would terminate"); + // Element-wise: `saved` holds a subset of the registry's tuple. template struct each; @@ -814,11 +829,22 @@ void registry::compiler::install_global_tables() { abort(); } - write_global_data(); - + // Report before installing, not after. Everything printed here is + // compiler-local - the report gathered during compile(), the slot + // assignment, the class lattices - so none of it needs the new tables to + // be in place; and all of it can throw, through the user-supplied `output` + // policy, or out of the containers print_slots() builds. Run after + // write_global_data() it would throw past the commit, leaving the new + // tables installed while initialize() never reaches + // `st.initialized = true` - a state the exception-safety contract does not + // describe, and one that makes the next call abort with `not_initialized` + // under runtime_checks. Before it, a throw is just another failure the + // transaction rolls back. print(report); print_slots(); ++tr << "Finished\n"; + + write_global_data(); } template diff --git a/test/compile_fail_policy_state_throwing_move.cpp b/test/compile_fail_policy_state_throwing_move.cpp new file mode 100644 index 00000000..7f9ab823 --- /dev/null +++ b/test/compile_fail_policy_state_throwing_move.cpp @@ -0,0 +1,62 @@ +// Copyright (c) 2017-2026 Jean-Louis Leroy +// Distributed under the Boost Software License, Version 1.0. +// See accompanying file LICENSE_1_0.txt +// or copy at http://www.boost.org/LICENSE_1_0.txt) + +// Expected diagnostic, as a CMake regex (see CMakeLists.txt). +// expected-error: nothrow move-assignable + +#include +#include + +#include +#include + +using namespace boost::openmethod; + +// initialize() saves this policy's state and puts it back, from the +// transaction's destructor, if another policy throws. That destructor runs +// while an exception is in flight and is noexcept by default, so a +// move-assignment that can throw would terminate the program instead of +// letting the original error through. The transaction refuses the policy at +// compile time rather than leave that in the program. +struct throwing_move_policy { + using category = throwing_move_policy; + + template + struct fn { + struct state { + state() = default; + state(const state&) = default; + auto operator=(const state&) -> state& = default; + + state(state&&) { + throw std::runtime_error("move"); + } + + auto operator=(state&&) -> state& { + throw std::runtime_error("move"); + } + + int generation = 0; + }; + + template + static void initialize(const Context&, const std::tuple&) { + ++Registry::template state().generation; + } + }; +}; + +struct bad_registry : default_registry::with {}; + +struct Animal { + virtual ~Animal() = default; +}; + +BOOST_OPENMETHOD_REGISTER(use_classes); + +int main() { + initialize(); + return 0; +} diff --git a/test/test_initialize_transaction.cpp b/test/test_initialize_transaction.cpp index 900038a1..0985198c 100644 --- a/test/test_initialize_transaction.cpp +++ b/test/test_initialize_transaction.cpp @@ -371,3 +371,40 @@ BOOST_AUTO_TEST_CASE(a_throwing_trace_does_not_commit) { BOOST_TEST(st.initialized); BOOST_TEST(poke::fn(dog) == "silence bark"); } + +// Same again for the reporting. print(report) and print_slots() used to run +// after write_global_data() had committed, so a throw out of either - through +// the user's `output` policy, or out of the containers print_slots() builds - +// left the new tables installed while initialize() never reached +// `st.initialized = true`. That is a fourth outcome the exception-safety +// contract does not describe, and it makes the next call abort with +// `not_initialized` under runtime_checks even though the tables are fine. +BOOST_AUTO_TEST_CASE(a_throwing_report_does_not_commit) { + using Registry = tracing_registry<__COUNTER__>; + using vptr_state = typename snapshot::vptr_state; + + BOOST_OPENMETHOD_REGISTER(use_classes); + BOOST_OPENMETHOD_REGISTER(poke::override>); + BOOST_OPENMETHOD_REGISTER(poke::override>); + + Dog dog; + auto& st = Registry::state(); + + initialize(); + BOOST_TEST(poke::fn(dog) == "silence bark"); + + snapshot before; + + trapping_stream::trap = "Used slots"; + BOOST_CHECK_THROW(initialize(trace(true)), std::runtime_error); + BOOST_TEST(trapping_stream::trap == nullptr); // it did throw there + + BOOST_TEST(!st.initialized); + BOOST_TEST(st.dispatch_data.data() == before.dispatch_data); + BOOST_TEST(Registry::template static_vptr == before.dog_vptr); + BOOST_TEST((detail::get(st.policies).vptrs == before.vptrs())); + + initialize(); + BOOST_TEST(st.initialized); + BOOST_TEST(poke::fn(dog) == "silence bark"); +} From 35f4e397364ca75cf4ed7b669e99d5355898bbd8 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Sat, 12 Sep 2026 10:27:52 -0400 Subject: [PATCH 5/5] initialize: say the state is preserved, not that it is consistent Three places claimed that a failed initialize() leaves the previous dispatch state "complete and consistent", or that "the v-table pointers, `next` pointers and dispatch tables from the previous call all stay in place". Both are false when the preceding call was finalize(): that clears the dispatch data and every policy's state but leaves the classes' static_vptrs set - as documented on static_vptr, which stays valid only until the next initialize() *or finalize()*. After initialize -> finalize -> failed initialize the two halves disagree, and no call is possible until an initialize() succeeds. What the transaction actually guarantees is preservation, not consistency: the call leaves the state it found, whatever that was. Say that instead. The guarantee is unchanged - only the description was too strong. Adds the case to the test, which the suite did not cover: finalize appeared in it only after a failed initialize, never before one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JQa4fuiwcfsheZYTCyfPPr --- .../ROOT/pages/registries_and_policies.adoc | 4 +- include/boost/openmethod/initialize.hpp | 11 ++-- test/test_initialize_transaction.cpp | 51 +++++++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/doc/modules/ROOT/pages/registries_and_policies.adoc b/doc/modules/ROOT/pages/registries_and_policies.adoc index a691c415..d3fa90a6 100644 --- a/doc/modules/ROOT/pages/registries_and_policies.adoc +++ b/doc/modules/ROOT/pages/registries_and_policies.adoc @@ -158,7 +158,9 @@ cpp:initialize[] is transactional. If a policy's `initialize` throws - `fast_perfect_hash` failing to find hash factors under `throw_error_handler`, say - every policy gets its previous state back, and nothing else in the registry is modified: the v-table pointers, `next` pointers and dispatch -tables from the previous call all stay in place. The registry is marked as not +tables are left exactly as the call found them. That need not be a state the +registry can dispatch through - after a cpp:finalize[] it is a torn-down one - +only the state that was there before. The registry is marked as not initialized, though, since that state no longer reflects the registrations, and cpp:initialize[] must be called again before calling a method. Only a registry with the cpp:runtime_checks[] policy diagnoses a call made in the diff --git a/include/boost/openmethod/initialize.hpp b/include/boost/openmethod/initialize.hpp index 977da500..a3e4f352 100644 --- a/include/boost/openmethod/initialize.hpp +++ b/include/boost/openmethod/initialize.hpp @@ -864,8 +864,11 @@ template template void registry::compiler::initialize() { // Clear the flag up front, and set it only once everything has succeeded. - // A run that throws leaves the previous dispatch state in place, complete - // and consistent (see write_global_data()) - but not marked initialized: + // A run that throws leaves the dispatch state exactly as the call found it + // (see write_global_data()) - which is not the same as a working one: if a + // finalize() came in between, what the call found was already torn down, + // the classes' static_vptrs still set and the dispatch data cleared. + // Either way it is not marked initialized: // those tables do not reflect the registrations that prompted the call, // and after a dlclose (the documented re-initialize flow) they may point // into unloaded code, so require_initialized() must keep refusing to @@ -1851,8 +1854,8 @@ void registry::compiler::write_global_data() { // Only then are the shared locations patched - the method_infos' slots // and strides, the overriders' `next`, the class_infos' static_vptr - and // the dispatch data swapped in, by commit_global_data(), which is - // `noexcept`. If a policy throws, the registry still holds the previous - // dispatch state, complete and consistent, rather than pointers into a + // `noexcept`. If a policy throws, the registry still holds the dispatch + // state it had on entry - whatever that was - rather than pointers into a // vector that unwinding has just freed. auto dispatch_data_size = std::accumulate( diff --git a/test/test_initialize_transaction.cpp b/test/test_initialize_transaction.cpp index 0985198c..29b8b078 100644 --- a/test/test_initialize_transaction.cpp +++ b/test/test_initialize_transaction.cpp @@ -408,3 +408,54 @@ BOOST_AUTO_TEST_CASE(a_throwing_report_does_not_commit) { BOOST_TEST(st.initialized); BOOST_TEST(poke::fn(dog) == "silence bark"); } + +// "The state the call found" is meant literally: it is not necessarily a state +// the registry can dispatch through. finalize() clears the dispatch data and +// every policy's state but leaves the classes' static_vptrs set - documented +// on static_vptr, which remains valid only until the next initialize() *or +// finalize()*. A failed initialize() after that restores exactly that +// half-torn-down state, which is why the guarantee is worded as preservation +// and not as consistency. +BOOST_AUTO_TEST_CASE_TEMPLATE( + failed_initialize_after_finalize_restores_what_it_found, Registry, + registries<__COUNTER__>) { + using explosive = typename explosive_policy::template fn< + typename Registry::registry_type>; + using vptr_state = typename snapshot::vptr_state; + + BOOST_OPENMETHOD_REGISTER(use_classes); + BOOST_OPENMETHOD_REGISTER( + typename poke::template override>); + BOOST_OPENMETHOD_REGISTER( + typename poke::template override>); + + Dog dog; + auto& st = Registry::state(); + + initialize(); + BOOST_TEST(poke::fn(dog) == "silence bark"); + + finalize(); + BOOST_TEST(!st.initialized); + BOOST_TEST(st.dispatch_data.empty()); + BOOST_TEST(detail::get(st.policies).vptrs.empty()); + // Not cleared by finalize, and so still set here. + auto dog_vptr_after_finalize = Registry::template static_vptr; + BOOST_TEST(dog_vptr_after_finalize != nullptr); + + explosive::armed = true; + BOOST_CHECK_THROW(initialize(), std::runtime_error); + explosive::armed = false; + + // Everything is put back the way the failed call found it - torn down, not + // consistent. + BOOST_TEST(!st.initialized); + BOOST_TEST(st.dispatch_data.empty()); + BOOST_TEST(detail::get(st.policies).vptrs.empty()); + BOOST_TEST(Registry::template static_vptr == dog_vptr_after_finalize); + + // And a successful call still recovers from it. + initialize(); + BOOST_TEST(st.initialized); + BOOST_TEST(poke::fn(dog) == "silence bark"); +}