fix(instrumented-pool): correctness fixes (data race, live underflow, move-assign event)

CHANGELOG.md

@@ -40,6 +40,27 @@ dated version block (`## [X.Y.Z] — YYYY-MM-DD`) when a release PR closes a mil
   back to `translated`, clearing the `i18n-freshness` flag the `v1.1.1` release raised.
   Documentation-only; no API change.
 
+### Fixed
+
+- **`InstrumentedPool` data race on the growth counter ([BUG-0001](docs/bugs/2026/06/BUG-0001-instrumented-pool-growth-counter-data-race.md)).**
+  `notify_if_grew()` read and wrote the non-atomic `last_growths_` on the allocation
+  hot path, while the decorator is documented as safe to drive concurrently over a
+  thread-safe pool — a data race (UB) under `MUTEX` + dynamic growth. `last_growths_`
+  is now `std::atomic` and advanced with a `compare_exchange`, so the growth event is
+  emitted once per growth without a race. A concurrent `InstrumentedPool` case was
+  added to the ThreadSanitizer stress suite. Header-only; no API change.
+- **`InstrumentedPool::deallocate` no longer underflows `live_` ([BUG-0002](docs/bugs/2026/06/BUG-0002-instrumented-pool-live-counter-underflow.md)).**
+  A foreign or double-freed pointer (a no-op in the core, ADR-0012) used to decrement
+  the unsigned `live_` counter unconditionally, wrapping it to `SIZE_MAX` and
+  corrupting `stats()`. The decrement now clamps at zero. The C header's
+  `memory_pool_free` note was corrected to stop claiming the Decorator *detects*
+  double-free (it counts but cannot distinguish one). Header/doc-only; no API change.
+- **`InstrumentedPool` move-assignment now emits `destroyed` for the replaced pool
+  ([BUG-0003](docs/bugs/2026/06/BUG-0003-instrumented-pool-move-assign-missing-destroyed-event.md)).**
+  Move-assigning over an instrumented pool released its `Pool` and observers without
+  notifying `PoolEvent::destroyed`, asymmetric with the destructor. It now notifies
+  before reassignment. Header-only; no API change.
+
 ## Released versions
 
 Each released version is an **immutable** entry under [`docs/changelog/`](docs/changelog/) — one file per release, newest first ([ADR-0038](docs/adr/0038-changelog-version-split.md)). They are never edited after release; only the `Unreleased` block above changes during development.

docs/bugs/2026/06/BUG-0001-instrumented-pool-growth-counter-data-race.md

@@ -0,0 +1,68 @@
+---
+id: BUG-0001
+title: Data race on InstrumentedPool::last_growths_ under concurrent use
+status: fixed
+severity: high
+reporter: third-party
+discovered: 2026-06-15
+affected-versions: ">=0.6.0,<1.1.2"
+fixed-in: v1.1.2
+---
+
+# BUG-0001: Data race on InstrumentedPool::last_growths_ under concurrent use
+
+## Summary
+
+`InstrumentedPool::notify_if_grew()` read **and wrote** the non-atomic member
+`last_growths_` on the hot allocation path, while the class contract explicitly
+permits driving the decorator concurrently over a thread-safe pool — a data race
+(C++ memory-model UB).
+
+## Environment
+
+- **Affected versions:** `>=0.6.0,<1.1.2` (`InstrumentedPool` landed in Milestone 6).
+- **Configuration:** `PBR_MEMORY_POOL_THREAD_SAFETY = MUTEX` **and** a dynamic
+  (growing) pool, driven from multiple threads. Not reachable under `LOCKFREE`
+  (dynamic growth is disabled there, ADR-0024 §2, so the growth counter stays 0)
+  nor under `NONE` (single-threaded by contract).
+
+## Reproduction
+
+Wrap a dynamic `MUTEX` pool in an `InstrumentedPool` and allocate from several
+threads so the pool grows repeatedly. `notify_if_grew()` — called from
+`allocate()` / `try_allocate()` — does `if (growths > last_growths_) { last_growths_ = growths; … }`
+on a plain `std::size_t`, concurrently. ThreadSanitizer flags the race; the
+pre-fix `concurrency_stress_test` did not exercise `InstrumentedPool` at all.
+
+## Expected vs. actual
+
+- **Expected:** the contract says the decorator "is safe to wrap a thread-safe
+  (`MUTEX`/`LOCKFREE`) pool and drive it concurrently".
+- **Actual:** concurrent allocations race on `last_growths_` — undefined behaviour
+  (a data race is UB regardless of the "approximate metrics" caveat, which only
+  excuses the *value*, not the race).
+
+## Root cause
+
+`last_growths_` was a plain `std::size_t` while every other counter was a
+`std::atomic`. The growth-detection read-modify-write was therefore unsynchronized.
+
+## Impact
+
+UB in a Tier-1 supported configuration (`MUTEX` + dynamic growth + concurrency +
+instrumentation). High severity: the unsafe access is on the allocation hot path.
+
+## Fix / workaround
+
+Made `last_growths_` a `std::atomic<std::size_t>`; `notify_if_grew()` now advances
+it with a relaxed `compare_exchange_weak`, so the winning thread notifies `grew`
+exactly once per observed growth and concurrent callers neither race nor
+double-notify. The move ctor/assignment load/store it atomically. Added an
+`InstrumentedPool`-over-dynamic-pool concurrency case to `concurrency_stress_test`
+(run under the MUTEX ThreadSanitizer CI job).
+
+## References
+
+- Fixing change: branch `fix/instrumented-pool-correctness` — see the `CHANGELOG` `Fixed` entry.
+- [`instrumented_pool.hpp`](../../../../src/main/cpp/it/d4np/memorypool/instrumented_pool.hpp) — `notify_if_grew`, `last_growths_`.
+- [ADR-0025](../../../adr/0025-decorator-for-instrumented-pool.md) (Decorator) · [ADR-0026](../../../adr/0026-observer-for-pool-lifecycle-events.md) (Observer / growth event) · [ADR-0024](../../../adr/0024-dynamic-growth-synchronization-and-creation-surface.md) (LOCKFREE never grows).

docs/bugs/2026/06/BUG-0002-instrumented-pool-live-counter-underflow.md

@@ -0,0 +1,75 @@
+---
+id: BUG-0002
+title: InstrumentedPool::deallocate underflows live_ on a foreign or double-freed pointer
+status: fixed
+severity: medium
+reporter: third-party
+discovered: 2026-06-15
+affected-versions: ">=0.6.0,<1.1.2"
+fixed-in: v1.1.2
+---
+
+# BUG-0002: InstrumentedPool::deallocate underflows live_ on a foreign or double-freed pointer
+
+## Summary
+
+`InstrumentedPool::deallocate()` unconditionally decremented the unsigned `live_`
+counter for any non-null pointer, even though the core `memory_pool_free` silently
+ignores foreign / misaligned pointers (ADR-0012). A foreign pointer (or a
+double-free) therefore drove `live_` below zero, **wrapping to `SIZE_MAX`** and
+corrupting `stats()` / `write_summary()`.
+
+## Environment
+
+- **Affected versions:** `>=0.6.0,<1.1.2`.
+- **Configuration:** any; triggered by a caller passing a non-null pointer that the
+  core treats as a no-op (out-of-range / misaligned / already-freed).
+
+## Reproduction
+
+```cpp
+InstrumentedPool pool{Pool(64, 4)};
+void* a = pool.try_allocate();
+pool.deallocate(a);          // live_ -> 0
+int stack_var = 0;
+pool.deallocate(&stack_var); // foreign: core no-op, but live_ decremented -> SIZE_MAX
+// pool.stats().live_ == SIZE_MAX
+```
+
+## Expected vs. actual
+
+- **Expected:** `live_` reflects outstanding blocks and never underflows; a free the
+  core rejects must not corrupt the diagnostic.
+- **Actual:** `live_` underflowed to `SIZE_MAX`; the summary reported absurd values.
+
+## Root cause
+
+The counter update was gated only on `block != nullptr`, not on whether the core
+actually accepted the free — and the core gives no acceptance signal. `live_` being
+`std::size_t`, the over-decrement wrapped.
+
+Related documentation defect: the C header (`memory_pool.h`) claimed the Decorator
+*addresses* double-free. It does not — it counts deallocations but cannot
+distinguish a double-free from a legitimate free. The header wording was corrected
+in the same change (the chosen scope is to fix the counter and tell the truth, not
+to implement double-free detection — that would be a separate feature).
+
+## Impact
+
+Corrupted diagnostics (not memory corruption — the core stays safe). Medium: it
+surfaces only on caller misuse, but the wrong-by-`SIZE_MAX` counter is badly
+misleading, and the header over-promised.
+
+## Fix / workaround
+
+`deallocate()` now decrements `live_` with a clamp-at-zero `compare_exchange_weak`
+loop, so a rejected/foreign/double free can never underflow it. `deallocations_`
+still counts every non-null call (its documented meaning). The header's double-free
+note was rewritten to stop promising Decorator detection.
+
+## References
+
+- Fixing change: branch `fix/instrumented-pool-correctness` — see the `CHANGELOG` `Fixed` entry.
+- [`instrumented_pool.hpp`](../../../../src/main/cpp/it/d4np/memorypool/instrumented_pool.hpp) — `deallocate`.
+- [`memory_pool.h`](../../../../src/main/cpp/it/d4np/memorypool/memory_pool.h) — `memory_pool_free` double-free note.
+- [ADR-0012](../../../adr/0012-foreign-pointer-and-out-of-range-pointer-policy.md) (foreign-pointer no-op) · [ADR-0025](../../../adr/0025-decorator-for-instrumented-pool.md) (Decorator).

docs/bugs/2026/06/BUG-0003-instrumented-pool-move-assign-missing-destroyed-event.md

@@ -0,0 +1,64 @@
+---
+id: BUG-0003
+title: InstrumentedPool move-assignment does not notify destroyed for the replaced pool
+status: fixed
+severity: low
+reporter: third-party
+discovered: 2026-06-15
+affected-versions: ">=0.6.0,<1.1.2"
+fixed-in: v1.1.2
+---
+
+# BUG-0003: InstrumentedPool move-assignment does not notify destroyed for the replaced pool
+
+## Summary
+
+`InstrumentedPool::operator=(InstrumentedPool&&)` released the destination's pool
+and overwrote its observer list without emitting `PoolEvent::destroyed` for the
+pool being replaced. Observers attached to the destination's prior contents
+silently lost their subject — asymmetric with the destructor, which does notify.
+
+## Environment
+
+- **Affected versions:** `>=0.6.0,<1.1.2`.
+- **Configuration:** any; triggered by move-assigning onto an `InstrumentedPool`
+  that already has registered observers.
+
+## Reproduction
+
+```cpp
+RecordingObserver obs;
+InstrumentedPool dest{Pool(64, 4)};
+dest.add_observer(obs);
+dest = InstrumentedPool{Pool(64, 4)};  // replaces dest's pool
+// before the fix: obs.destroyed_ == 0 (the replaced pool went away silently)
+```
+
+## Expected vs. actual
+
+- **Expected:** the pool being replaced is going away, so its observers should see
+  `destroyed` — the same guarantee the destructor gives.
+- **Actual:** no `destroyed` event was delivered on move-assignment.
+
+## Root cause
+
+`operator=` overwrote `pool_` and `observers_` without first notifying the existing
+observers. Not a leak (the moved-into `Pool` is properly released), but a
+lifecycle-event asymmetry.
+
+## Impact
+
+Low / cosmetic: observers relying on a balanced `destroyed` per subject miss one on
+move-assignment. No memory-safety consequence.
+
+## Fix / workaround
+
+`operator=` now calls `notify(PoolEvent::destroyed)` at the top of the
+self-assignment-guarded block, before the pool and observer list are overwritten —
+symmetric with the destructor. Covered by a new move-assignment observer test.
+
+## References
+
+- Fixing change: branch `fix/instrumented-pool-correctness` — see the `CHANGELOG` `Fixed` entry.
+- [`instrumented_pool.hpp`](../../../../src/main/cpp/it/d4np/memorypool/instrumented_pool.hpp) — `operator=`, `~InstrumentedPool`.
+- [ADR-0026](../../../adr/0026-observer-for-pool-lifecycle-events.md) (Observer lifecycle events).

docs/bugs/README.md

@@ -73,11 +73,12 @@ checked by `python tools/consistency_lint.py` (the `bugs` check).
 
 ## Index
 
-Newest first, grouped by year and month. *(No defects recorded yet — the ledger is empty
-as of `v1.1.0`.)*
+Newest first, grouped by year and month.
+
+### 2026 — June
 
-<!-- When adding a record, append a row here:
 | Id | Title | Status | Severity | Discovered |
 |----|-------|--------|----------|------------|
-| [BUG-0001](2026/06/2026...) | ... | open | medium | 2026-06-15 |
--->
+| [BUG-0001](2026/06/BUG-0001-instrumented-pool-growth-counter-data-race.md) | Data race on `InstrumentedPool::last_growths_` under concurrent use | `fixed` | high | 2026-06-15 |
+| [BUG-0002](2026/06/BUG-0002-instrumented-pool-live-counter-underflow.md) | `InstrumentedPool::deallocate` underflows `live_` on a foreign / double-freed pointer | `fixed` | medium | 2026-06-15 |
+| [BUG-0003](2026/06/BUG-0003-instrumented-pool-move-assign-missing-destroyed-event.md) | `InstrumentedPool` move-assignment does not notify `destroyed` for the replaced pool | `fixed` | low | 2026-06-15 |

src/main/cpp/it/d4np/memorypool/instrumented_pool.hpp

@@ -131,11 +131,14 @@ class InstrumentedPool {
           allocation_failures_(other.allocation_failures_.load(std::memory_order_relaxed)),
           live_(other.live_.load(std::memory_order_relaxed)),
           peak_live_(other.peak_live_.load(std::memory_order_relaxed)), observers_(std::move(other.observers_)),
-          last_growths_(other.last_growths_) {}
+          last_growths_(other.last_growths_.load(std::memory_order_relaxed)) {}
 
-    /** Move-assign; releases the current pool and re-seeds the counters + observers. */
+    /** Move-assign; releases the current pool and re-seeds the counters + observers.
+     *  The pool being replaced is going away, so its observers get a `destroyed`
+     *  event first — symmetric with the destructor (BUG-0003). */
     InstrumentedPool& operator=(InstrumentedPool&& other) noexcept {
         if (this != &other) {
+            notify(PoolEvent::destroyed);
             pool_ = std::move(other.pool_);
             allocations_.store(other.allocations_.load(std::memory_order_relaxed), std::memory_order_relaxed);
             deallocations_.store(other.deallocations_.load(std::memory_order_relaxed), std::memory_order_relaxed);
@@ -144,7 +147,7 @@ class InstrumentedPool {
             live_.store(other.live_.load(std::memory_order_relaxed), std::memory_order_relaxed);
             peak_live_.store(other.peak_live_.load(std::memory_order_relaxed), std::memory_order_relaxed);
             observers_ = std::move(other.observers_);
-            last_growths_ = other.last_growths_;
+            last_growths_.store(other.last_growths_.load(std::memory_order_relaxed), std::memory_order_relaxed);
         }
         return *this;
     }
@@ -191,7 +194,13 @@ class InstrumentedPool {
     void deallocate(void* block) noexcept {
         if (block != nullptr) {
             deallocations_.fetch_add(1U, std::memory_order_relaxed);
-            live_.fetch_sub(1U, std::memory_order_relaxed);
+            // Clamp at zero: a foreign or already-freed pointer is a no-op in the
+            // core (ADR-0012), so an unconditional fetch_sub would underflow `live_`
+            // to SIZE_MAX on such a call (BUG-0002). Decrement only while positive.
+            std::size_t live = live_.load(std::memory_order_relaxed);
+            while (live > 0U && !live_.compare_exchange_weak(live, live - 1U, std::memory_order_relaxed)) {
+                // `live` is reloaded by compare_exchange_weak on failure; retry.
+            }
         }
         pool_.deallocate(block);
     }
@@ -250,9 +259,16 @@ class InstrumentedPool {
      *  the dynamic pool grew, so notify `grew`. */
     void notify_if_grew() noexcept {
         const std::size_t growths = ::memory_pool_growths(pool_.native_handle());
-        if (growths > last_growths_) {
-            last_growths_ = growths;
-            notify(PoolEvent::grew);
+        // Advance the high-water growth count atomically; the thread that wins the
+        // compare-exchange notifies `grew` exactly once per observed growth, so
+        // concurrent callers neither race on the counter nor double-notify (BUG-0001).
+        std::size_t last = last_growths_.load(std::memory_order_relaxed);
+        while (growths > last) {
+            if (last_growths_.compare_exchange_weak(last, growths, std::memory_order_relaxed)) {
+                notify(PoolEvent::grew);
+                break;
+            }
+            // `last` is reloaded by compare_exchange_weak on failure; re-check.
         }
     }
 
@@ -263,7 +279,9 @@ class InstrumentedPool {
     std::atomic<std::size_t> live_{0U};
     std::atomic<std::size_t> peak_live_{0U};
     std::vector<PoolObserver*> observers_;
-    std::size_t last_growths_{0U};
+    // Atomic: read+advanced on the hot allocation path (`notify_if_grew`), which the
+    // class contract allows to run concurrently over a thread-safe pool (BUG-0001).
+    std::atomic<std::size_t> last_growths_{0U};
 };
 
 }  // namespace it::d4np::memorypool

src/main/cpp/it/d4np/memorypool/memory_pool.h

@@ -163,8 +163,10 @@ void* memory_pool_alloc(memory_pool_t* pool);
  * pool state is bit-identical before and after a no-op call. Note that
  * the policy does NOT detect *double-free* on a legitimately-in-range,
  * already-on-the-free-list pointer; that case remains undefined
- * behaviour and is addressed by the optional Decorator instrumentation
- * landing in Milestone 6.
+ * behaviour. The optional `InstrumentedPool` Decorator (Milestone 6)
+ * counts deallocation calls but does not detect a double-free either —
+ * it cannot distinguish one from a legitimate free — so it must not be
+ * relied on to make a double-free safe.
  *
  * @param pool  Pool the block originally came from.
  * @param block Block to release, or `NULL`, or a foreign pointer.