Record concurrency and lifetime audit results
This commit is contained in:
108
docs/roadmap/concurrency.md
Normal file
108
docs/roadmap/concurrency.md
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user