fix(instrumented-pool): correctness fixes (data race, live underflow, move-assign event)#99
Merged
Conversation
… move-assign event) Three verified third-party-reported defects in the InstrumentedPool Decorator (ADR-0025/0026), recorded in the bug ledger (ADR-0039) and fixed for v1.1.2. No public API change; header-only. - BUG-0001 (high): notify_if_grew() read+wrote the non-atomic last_growths_ on the allocation hot path, racing under the documented-safe MUTEX + dynamic-growth + concurrent configuration (UB). Make last_growths_ std::atomic and advance it with a compare_exchange so `grew` fires once per growth without a race. Add a concurrent InstrumentedPool case to the TSan stress suite. - BUG-0002 (medium): deallocate() decremented the unsigned live_ for any non-null pointer, so a foreign/double-freed pointer (a core no-op, ADR-0012) underflowed it to SIZE_MAX. Clamp the decrement at zero. Correct the memory_pool.h double-free note: the Decorator counts but does not detect a double-free (it cannot distinguish one) — it no longer claims to address it. - BUG-0003 (low): move-assignment released the destination pool and observers without emitting PoolEvent::destroyed, asymmetric with the destructor. Notify before reassignment. Verified locally with clang-format / clang-tidy (headers + both test TUs compile clean, incl. the MUTEX-gated concurrency case) and the consistency lint; CI runs the full matrix + ASan/UBSan/TSan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
7 tasks
danielPoloWork
added a commit
that referenced
this pull request
Jun 15, 2026
…rflow (#100) ## Summary Closes the latent `size_t` overflow in `grow_pool`'s growth-size computation (BUG-0004) — the last of the four verified third-party-reported defects. Core-only, no API change. ## Motivation `grow_pool` computed `total * (grow_factor_ - 1)` **before** any overflow check, then validated `add * block_size_` — so the first multiplication could wrap `size_t` and feed the downstream guard an already-wrapped value. Inconsistent with the meticulous overflow handling on the create path (ADR-0009 §3). Record: [BUG-0004](docs/bugs/2026/06/BUG-0004-grow-pool-growth-size-overflow.md). ## Changes - `memory_pool.cpp` (`grow_pool`) — add `would_overflow_product(total, grow_factor_ - 1)` guard before computing `add`; on overflow the pool falls back to fixed-mode exhaustion, exactly as the existing guards do. - Bug ledger record BUG-0004 (`fixed`, `fixed-in: v1.1.2`) + index; `CHANGELOG` `Fixed` entry. ## Design Patterns - None — a defensive guard within the existing dynamic-growth path (ADR-0022/0024). ## Verification - [x] `clang-format` + `clang-tidy` clean on `memory_pool.cpp` (`grow_pool` compiles under the default build) - [x] `python tools/consistency_lint.py` — OK; `markdownlint-cli2` clean - [x] No test added — the overflow is **not runtime-reachable** through the public API (RAM exhausts long before `total` nears the boundary); the record documents why - [ ] Full build matrix — CI ## Documentation Impact - [x] Bug ledger record + index; `CHANGELOG` `Fixed` - [x] No README / ROADMAP / ADR change (maintenance fix; PATCH per maintenance.md) - [x] PR metadata set — assignee, `fix` label, `v1.1.2` milestone (§6.4 / ADR-0040) ## Context Second and final fix PR for the four reported defects (PR #99 covered the three `InstrumentedPool` issues). After this merges, `v1.1.2` is ready to cut whenever you choose. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
danielPoloWork
added a commit
that referenced
this pull request
Jun 15, 2026
## Summary Cuts the **v1.1.2** maintenance release — a **PATCH** bundling the four verified bug fixes (BUG-0001…0004, the first real use of the in-repo bug ledger) and the documentation work accumulated since `v1.1.1`. The library's public surface is unchanged (no API/ABI change). Full notes: [`docs/releases/v1.1.2.md`](docs/releases/v1.1.2.md). ## Motivation SemVer PATCH per the [maintenance decision tree](docs/workflow/maintenance.md) (no public-surface change). Mechanics per [`docs/workflow/release.md`](docs/workflow/release.md). ## Changes - **`version.hpp`** — PATCH `1 → 2`, STRING `"1.1.1" → "1.1.2"`; `pool_smoke` version `TEST_CASE` updated. - **`CHANGELOG.md`** — `[Unreleased]` rolled into the immutable [`docs/changelog/v1/v1.1.2.md`](docs/changelog/v1/v1.1.2.md) (links re-based), `Unreleased` reset, index row added, compare link → `v1.1.2...HEAD`. - **`docs/releases/v1.1.2.md`** — draft GitHub Release notes. - **`README.md`** — status badge → `v1.1.2` + a new `v1.1.2` status paragraph. - **i18n** — the two README translation rows marked `stale` (English README changed); the `zh-Hans`/`ja` re-sync is a follow-up. The `i18n-freshness` lint skips `stale` rows, so the gate stays green. ## Release contents **Fixed:** BUG-0001 (InstrumentedPool growth-counter data race), BUG-0002 (`live_` underflow + header doc), BUG-0003 (move-assign `destroyed` event), BUG-0004 (`grow_pool` overflow guard). **Removed:** the `docs-site` README badge. **Changed:** `zh-Hans`/`ja` README re-synced to v1.1.1. ADR total stays 40. ## Design Patterns - None — release bookkeeping. ## Verification - [x] `python tools/consistency_lint.py` — OK (version-lockstep sees `version.hpp`/CHANGELOG/README badge/release notes all at `1.1.2`; `i18n-freshness` green) - [x] `markdownlint-cli2` + `clang-format` clean - [ ] CI build matrix + ASan/UBSan/TSan — validated on this PR (the bug fixes themselves landed and passed CI in #99/#100) ## Documentation Impact - [x] README badge + paragraph; CHANGELOG rolled; release notes added - [x] ROADMAP — N/A (maintenance PATCH, no milestone change) - [x] PR metadata set — assignee, `chore` label, `v1.1.2` milestone (§6.4 / ADR-0040) ## After merge The agent tags `v1.1.2` from `master` ([ADR-0008](docs/adr/0008-delegate-tag-creation-and-push-to-the-agent.md)); `release.yml` produces a **draft** GitHub Release; the maintainer clicks *Publish*. Then a follow-up PR re-syncs the `zh-Hans`/`ja` README translations to `v1.1.2` and closes the `v1.1.2` milestone. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three verified, third-party-reported defects in the
InstrumentedPoolDecorator (ADR-0025/0026), recorded in the bug ledger (ADR-0039) and targeted at thev1.1.2PATCH. No public API change — header-only + a C-header doc correction.Motivation
Reported externally; I reproduced/root-caused each against the code and the documented contracts before accepting (the §7.7 triage flow). All three confirmed real and not excused by documented behaviour. Records: BUG-0001, BUG-0002, BUG-0003.
Changes
notify_if_grew()read+wrote the non-atomiclast_growths_on the allocation hot path, racing under the documented-safeMUTEX+ dynamic-growth + concurrent config (UB).last_growths_is nowstd::atomic, advanced with acompare_exchangesogrewfires once per growth without a race. Added a concurrentInstrumentedPoolcase to the ThreadSanitizer stress suite.deallocate()decremented the unsignedlive_for any non-null pointer, so a foreign/double-freed pointer (a core no-op, ADR-0012) wrapped it toSIZE_MAX. The decrement now clamps at zero. Thememory_pool.hdouble-free note was corrected — the Decorator counts but does not detect a double-free, so it no longer claims to address it.PoolEvent::destroyed, asymmetric with the destructor. It now notifies before reassignment.fixed,fixed-in: v1.1.2) + index;CHANGELOGFixedentries; two unit tests (clamp, move-assign event) + one concurrency test (BUG-0001).Design Patterns
Verification
clang-formatclean;clang-tidyclean — headers + both test TUs compile (incl. theMUTEX-gated concurrency case via-DPBR_MEMORY_POOL_THREAD_SAFETY=1)python tools/consistency_lint.py— OK (thebugscheck validates the 3 records)markdownlint-cli2cleanDocumentation Impact
CHANGELOGFixed; C-header doc correctedfixlabel,v1.1.2milestone (§6.4 / ADR-0040)Follow-up
A second PR (after this merges) fixes the latent core overflow in
grow_pool(the LOW-severity BUG-0004), keeping one component per PR.🤖 Generated with Claude Code