Skip to content

initialize: keep the previous dispatch state when a policy throws - #95

Open
jll63 wants to merge 2 commits into
boostorg:developfrom
jll63:fix/initialize
Open

initialize: keep the previous dispatch state when a policy throws#95
jll63 wants to merge 2 commits into
boostorg:developfrom
jll63:fix/initialize

Conversation

@jll63

@jll63 jll63 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

(Written by Claude Code, on behalf of @jll63.)

Fixes #81.

write_global_data() patched every class' static_vptr, every overrider's next and each method's slots and strides to point into a local vector, then ran the policies' initialize, and only then swapped the vector into the registry's state. A policy that threw - fast_perfect_hash failing to find hash factors under throw_error_handler, or a plain bad_alloc - left all of those pointers dangling into a vector that unwinding had just freed, and clobbered the dispatch state of the previous initialize. Re-initialize after dlopen/dlclose is the documented flow, so this is reachable.

Change

write_global_data() is now stage → policies → commit, with an RAII guard and no try/catch:

  • Stage: the dispatch data is built in a local vector, and each class' v-table pointer is staged in its class_ instead of being written to *ci->static_vptr.
  • Policies run inside detail::registry_state_transaction<Registry>, which copies the registry state's policies tuple whole on construction and moves it back on destruction unless commit() was called. The class and method lists need no saving (initialize only reads them), and dispatch_data must not be restored from a copy: the previous static_vptrs point into the live buffer, which is why it is only ever replaced at commit.
  • Commit, on a path where nothing can throw: slots and strides (and their propagation to other modules' copies), next pointers, static_vptr, then the dispatch_data swap.

A failed initialize() leaves the previous tables in place, complete and consistent, though not marked initialized: they do not reflect the registrations that prompted the call, and after a dlclose they may point into unloaded code, so require_initialized() keeps refusing to dispatch until a call succeeds.

The InitializeClass blueprint changes accordingly: vptr() returns the staged pointer by value, and a new static_vptr() returns the address that will receive it, for indirect_vptr policies. vptr_vector and vptr_map use it where they took &iter->vptr(); a user policy doing that now gets a compile error instead of a silently dangling pointer.

Test

test/test_initialize_transaction.cpp: two registries (vptr_vector; vptr_map + indirect_vptr) and a policy, placed after the vptr policy, whose initialize writes to its state then throws on demand. Asserts that the dispatch buffer, static_vptr<Dog/Cat>, next<poke_dog>, the hash range, the vptr policy's state and the throwing policy's own state are unchanged; that dispatch throws not_initialized; that a later initialize() succeeds; and that a failing first initialize() leaves everything null/empty and finalize() copes. Against the previous headers it fails 13 checks.

Verified with CMake (gcc, Debug: 148/148) and b2 (gcc: 87/87); docs rendered (Antora + MrDocs).

🤖 Generated with Claude Code

https://claude.ai/code/session_012QA4PCHE4oooe1ZtG9aKj7

write_global_data() patched every class' static_vptr, every overrider's
`next` and each method's slots and strides to point into a local vector,
then ran the policies' initialize, and only then swapped the vector into
the registry's state. A policy that threw - fast_perfect_hash failing to
find hash factors under throw_error_handler, or a plain bad_alloc - left
all of those pointers dangling into a vector that unwinding had just
freed, and clobbered the dispatch state of the previous initialize.

Reorder it into stage, policies, commit. The dispatch data is built in a
local vector and each class' v-table pointer is staged in its class_,
where the policies read it; the policies run with their states saved in
an RAII transaction that puts them back if one throws; the shared
locations are written only after that, and nothing on that path can
throw. A failed initialize() - a re-initialize after dlopen/dlclose
included - leaves the previous tables in place, complete and
consistent, though not marked initialized.

The InitializeClass blueprint changes accordingly: vptr() returns the
staged pointer by value, and static_vptr() returns the address that will
receive it, for the policies that store pointers to v-table pointers.

Fixes boostorg#81.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012QA4PCHE4oooe1ZtG9aKj7
@cppalliance-bot

cppalliance-bot commented Sep 1, 2026

Copy link
Copy Markdown

An automated preview of the documentation is available at https://95.openmethod.prtest3.cppalliance.org/libs/openmethod/doc/html/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-09-01 23:26:58 UTC

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.83333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.09%. Comparing base (b3bec3b) to head (62515c7).
⚠️ Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
include/boost/openmethod/initialize.hpp 95.65% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #95      +/-   ##
===========================================
- Coverage    94.94%   93.09%   -1.85%     
===========================================
  Files           99       22      -77     
  Lines         4373     1666    -2707     
  Branches      2168      508    -1660     
===========================================
- Hits          4152     1551    -2601     
+ Misses         162       66      -96     
+ Partials        59       49      -10     
Files with missing lines Coverage Δ
include/boost/openmethod/policies/vptr_map.hpp 87.50% <100.00%> (-12.50%) ⬇️
include/boost/openmethod/policies/vptr_vector.hpp 100.00% <100.00%> (ø)
include/boost/openmethod/preamble.hpp 82.14% <ø> (+6.21%) ⬆️
include/boost/openmethod/initialize.hpp 92.12% <95.65%> (+0.13%) ⬆️

... and 86 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd2c86f...62515c7. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

BOOST_TEST(a == b) decomposes the comparison so that it can print both
operands when it fails. For the `next` pointers that means streaming a
function pointer, which is a Microsoft extension: clang-cl rejects it
under /WX (-Wmicrosoft-cast), so test_initialize_transaction did not
compile in any of the clang-win CI variants.

Wrap the comparison in an extra pair of parentheses, as the neighbouring
checks already do, so Boost.Test sees a bool.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VGX6XBwxEr1rqxGvAyvUgB
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

make initialize() transactional (write_global_data is not exception-safe)

2 participants