From 0cca633a67abd3d0fdcb7db6e83260dcaf1eaac8 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 25 Oct 2026 16:25:10 +0000 Subject: [PATCH] Record concurrency and lifetime audit results --- docs/roadmap/concurrency.md | 108 ++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 docs/roadmap/concurrency.md diff --git a/docs/roadmap/concurrency.md b/docs/roadmap/concurrency.md new file mode 100644 index 0000000..b468389 --- /dev/null +++ b/docs/roadmap/concurrency.md @@ -0,0 +1,108 @@ +# Concurrency And Lifetime Audit + +Reviewed revision: `a80273433e87ca955e5a5d9e99b616d8da10f185`. +Audit date: 2026-10-25. + +## Scope And Assessment + +This audit reviewed source reuse, concurrent independent loads, schema and +acquisition state, callback lifetime, publication, frozen metadata/explanation/ +diff/rendering, resource contexts, and secrets bootstrap. Knowledge-graph searches +and caller traces were supplemented by source inspection and existing tests. + +No new data race, deadlock, unsafe synchronization, goroutine leak, channel +management defect, or cancellation/shutdown defect was established under the +documented usage contracts. One previously reported callback-lifetime defect +remains reproducible and relevant to teardown; it is referenced below rather +than assigned a second independent remediation task. + +The [implemented architecture](../internal/overview.md) provides a straightforward +ownership model for concurrency: + +- Loading runs synchronously through one engine lifecycle. Production code + launches no library-owned goroutines and has no channel, worker, timer, watcher, + or background shutdown protocol to manage. +- Acquisition, version evidence, schema caches, reflection traversal, composition, + and callback records belong to a load or operation, rather than a shared global + cache. The root facade delegates without introducing a second shared state owner. +- Reader-source copies share a pointer to `readerSource`, whose atomic one-use + claim precedes reading. Byte and mapping sources retain constructor snapshots. +- Publication builds the retained inspection indexes before returning. Query, + explanation, diff, and rendering paths read frozen state and allocate their + own mutable results or replay trees. Typed configuration and returned metadata + are detached from the retained inspection snapshot. +- Returned resource contexts contain passive root information or a borrowed + filesystem. Host resource opens acquire their own handles, with ownership + transferred to the returned file, rather than sharing the acquisition handle. + +These observations support the existing model; they are not new blanket +thread-safety guarantees for application callbacks, borrowed objects, or I/O. + +## 1. Callback Lifetime Guards Survive Panic Unwinding — Previously Reported + +Status: open, tracked by +[correctness finding 4](correctness.md#4-callback-contexts-remain-active-after-panic-unwinding). +Priority: low. Category: callback teardown across the facade/engine boundary. + +**Relevant code.** `runResolution` in +[resolution.go](../../internal/engine/resolution.go), and `runValidation` / +`reportVersion` in [validation.go](../../internal/engine/validation.go), deactivate +the callback state only after a normal return. The root callback wrappers in +[lifecycle.go](../../lifecycle.go) expose that same engine context. + +**Circumstances and consequence.** If a callback retains its context and panics, +and application code recovers outside the load, the retained context still +accepts queries or diagnostic writes after callback exit. It should instead +produce the established closed-context failure. The abandoned per-load state +therefore remains accessible beyond its intended lifetime. Reusing the existing +external diagnostic probe reproduced this for resolution, semantic validation, +and version-check contexts at the reviewed revision. + +**Potential remediation.** Use invocation-scoped deferred deactivation, as +described in the linked correctness finding, preserving immediate closure on +normal return and unchanged propagation of application panics. There is no need +to add context locks, atomic lifecycle flags, panic recovery, or background-work +management. Concurrent use of a live callback context is already outside its +supported contract; this finding does not establish an additional supported-use +race or justify broadening that contract. + +## Validation + +The following passed with Go 1.26.10 on Linux amd64: + +```sh +go test -race ./... -count=1 +go vet ./... +``` + +The six existing engine concurrency tests for shared definitions, copied-reader +claims, concurrent inspection, selected-overlay loads, and resolution-input +isolation also passed with `-race -count=20 -cpu=1,4,8`. These tests live in +[independence_test.go](../../internal/engine/independence_test.go), +[source_test.go](../../internal/engine/source_test.go), +[explain_test.go](../../internal/engine/explain_test.go), and +[provenance_test.go](../../internal/engine/provenance_test.go). + +The previously created standalone callback-panic probe was rerun outside the +repository with race instrumentation. It confirmed the known lifetime defect +without reporting a data race; that probe performs sequential post-panic calls, +not unsupported concurrent callback-context use. No new tests or production +changes were made. Race detection covers executed paths and is not a proof of +all possible interleavings. No native macOS or Windows execution was performed. + +## Contract Boundaries Preserved + +- Read-only shared definitions and independent result ownership remain the + existing model. Callers remain responsible for synchronization of their own + mutable options, callback effects, readers, filesystems, and output writers. +- Secrets bootstrap is explicitly a startup operation with no environment + compare-and-set guarantee; its check-then-set behavior is not a new finding. + See the [bootstrap contract](../config.md#explicit-secrets-directory-bootstrap). +- Synchronous loading has no cancellation API or background refresh service. + Caller-supplied blocking operations are not evidence of a leaked ConfigKit + goroutine or a missing promised shutdown mechanism. See + [completion and resource lifetime](../config.md#completion-and-resource-lifetime). + +This report adds no synchronization requirements, new API, or release gates. +The existing [correctness](correctness.md), [efficiency](efficiency.md), and +[maintainability](complexity.md) findings retain their own scopes and status.