unhappy-path-audit

# Unhappy Path Audit Skill ## Core Logic (First Principles) **Unhappy path auditing = answering three propositions:** | Q | Proposition | Violation Consequence | |---|-------------|----------------------| | **Q1: Every creation has one effective destruction owner** | Ownership may transfer or be ref-counted, but each resource reaches cleanup and cleanup executes effectively once | **Leak / double cleanup** | | **Q2: Every wait dependency chain has a termination** | Every explicit or implicit blocking edge reaches a guaranteed release, cancellation, or bounded timeout | **Hung** | | **Q3: Every accumulation has an upper bound** | For every unbounded-growth container → capacity limit or recycling mechanism | **OOM** | **Key principle**: Not "looks like it might leak/hang", but "prove it definitely leaks/hangs". Q1 includes ownership cardinality, not only the presence of a destructor. Q2 includes mutex, channel, callback, RPC, I/O, and retry dependencies, not only functions named `Wait`. Exhaust every bypass and release path before ruling. --- ## Systematic Method (Drill-Down Algorithm) ``` Entry function └→ Layer 1: Execute Q1-Q3 on every resource/wait/accumulation point ├─ Termination condition satisfied at this layer → ✅, stop drill-down └─ Termination condition depends on a lower layer → ⬇ drill down to Layer 2 └→ Recurse with Q1-Q3 ``` ### Drill-Down Example ``` proxy.Close() └→ tunnel.Close() ← Q1: Destruction guaranteed? ✅ (defer) └→ pipe.kickoff() exit ← Q2: io.EOF guaranteed? → depends on connection.Close() └→ connection.Close() ← Q2: Close guaranteed? → depends on morpc readLoop detection └→ morpc readLoop ← Q2: Condition for detecting close? ├─ readTimeout > 0 → periodic return + detect → bounded edge └─ readTimeout = 0 → pending: does another owner call Close/cancel, and does that operation interrupt conn.Read? ``` A zero read timeout is not proof of a hang. It becomes a Q2 violation only when the complete ownership graph has no independent close/cancel edge that reliably interrupts the blocked read. --- ## Audit Workflow ### Phase 1 — Scope Definition ``` Input: target module list Actions: 1. Trace the complete call chain from entry to exit 2. Number layers: Layer 1, Layer 2, ... Layer N 3. Mark each layer's concern dimensions (goroutine/connection/lock/memory/...) 4. Draw resource ownership transfers and wait-for edges 5. Mark restart/retry/pool generation boundaries Output: audit scope matrix ``` ### Phase 2 — Per-Layer Audit ``` For each Layer: 1. Read a coherent batch of files (about six is a useful progress cadence, not an audit boundary); continue in further batches until the graph closes 2. Execute Q1-Q3 on this layer, including ownership cardinality and hidden wait edges 3. Record verdict + layer number where termination condition is satisfied 4. If termination depends on a lower layer → mark drill-down target, do not conclude at this layer 5. Output this layer's audit table → then proceed to the next layer ``` **Anti-false-positive rule**: For every wait point, first exhaust all release paths (including defer, cancel watcher, timer goroutine). Follow indirect edges such as `reject → mutex → owner → RPC`; a control path is not fail-fast merely because it returns an error after the owner eventually releases it. Only rule it hung when every path is confirmed unreachable. ### Phase 3 — Synthesis ``` 1. Merge all layer audit tables 2. Extract failed Q1/Q2/Q3 items 3. Verify termination chain integrity (no break from entry to exit) 4. Apply Bug Claim Verification Checklist (see Critical Rules) to each candidate 5. Generate issue-ready finding drafts for confirmed bugs; create external issues only when the user explicitly authorizes that write Output: final risk matrix + confirmed finding list ``` --- ## Output Format ### Per-Layer Audit Table (Phase 2 output) ``` Layer N: [Module Name] ([File Name]) | Q | Proposition | Code Path | Verdict | Termination Layer | |---|-------------|-----------|---------|-------------------| | Q1 | creation→destruction | go xxx() → defer close() | ✅ | Layer N | | Q2 | wait→termination | cond.Wait() → Broadcast | ✅ | Layer N (cancel watcher) | | Q3 | accumulation→bound | channel | ✅ | Layer N | ``` ### Final Risk Matrix (Phase 3 output) ``` | # | Bug | Layer | Q | Severity | Issue | |---|-----|-------|---|----------|-------| | 1 | [description] | Layer X | Q2 | High | issue-ready draft or authorized issue | ``` ### Termination Chain Verification (Phase 3 output) ``` Entry └→ Layer 1: destruction call ✅ └→ Layer 2: wait released ✅ └→ Layer 3: **BREAK** ← Q1 destruction unreachable ``` --- ## Critical Rules ### Core Rules 1. **Anti-confirmation-bias**: prove "definitely leaks/hangs", not "looks like it might" — exhaust all bypass paths before ruling 2. **Progress batching**: read coherent batches and report progress after roughly six files when the audit continues; never stop before the ownership/wait/growth graph closes 3. **Exit self-check**: before ending the turn confirm — ①conclusion in first paragraph ②audit table present ③each confirmed bug has an issue-ready draft; create an Issue only with authorization 4. **Drill-down discipline**: when termination condition is not satisfied, mark "pending drill-down", do not speculate at the current layer ### Bug Claim Verification Checklist (BEFORE filing ANY bug) **Every bug claim MUST pass all 5 gates before being included in the final report:** | # | Gate | Verification Action | Anti-Pattern (caught by spill audit) | |---|------|---------------------|--------------------------------------| | **G1** | **FULL-GRAPH** | Trace ownership to ALL terminal nodes and prove one effective cleanup owner on every path | Bug: found a destructor but missed a second concurrent owner, or stopped before the real terminal holder | | **G2** | **CAN-FAIL/BLOCK** | Open the alleged function and every wait-for dependency; confirm it can actually fail, panic, or block | Bug: called a path fail-fast without noticing its mutex owner was blocked in downstream I/O | | **G3** | **BOUND/RELEASE** | For every growth claim, trace retained live memory through scope/GC, `clear`/`delete`, truncation, eviction, backpressure, ownership transfer, generation replacement, and terminal cleanup; prove no reachable bound or release applies | Bug: saw `spillIndex = append(...)`, missed lifecycle cleanup or that ownership left the growing object | | **G4** | **LINE-REREAD** | Re-read every cited line number AFTER forming the bug hypothesis — do not trust memory from the initial read | Bug: asserted `ctr.state` doesn't become `SendSucceed` on failure; line 127 shows it's unconditional | | **G5** | **CALIBRATE-LAST** | Severity (Low/Medium/High) assigned ONLY after G1-G4 pass AND all bugs in the batch are confirmed. Never assign severity during discovery | Bug was Medium → turned out to be false positive (severity meaningless) | **Process rule**: G1-G4 are per-bug gates applied during Phase 2→3 transition. G5 is a global gate applied after the full bug list is finalized. Any bug failing G1-G4 is **discarded**, not downgraded. ### G1 Detail: Full-Graph Traversal For every resource (fd, lock, goroutine, memory allocation), construct the complete directed ownership graph: ``` Creation Point ──transfer──→ Holder A ──transfer──→ Holder B ──transfer──→ ... │ │ ▼ ▼ Reset() Destroy() Free() FreeMemory() ``` Required actions per node: 1. Identify every transfer function (hand-off, assignment, message send) 2. For each transfer target, locate its destruction path 3. Verify destruction path is reachable on error branches too 4. Prove competing branches cannot both execute irreversible cleanup, unless the destructor itself is verified idempotent 5. Repeat until reaching a node with NO further transfer (terminal) **Stop condition**: Graph is complete when every leaf is either (a) a verified destructor call, or (b) a GC-managed resource with bounded lifetime. ### Q2 Detail: Wait-For And Control Paths Construct a wait-for graph for each potentially blocking path: ``` caller -> mutex/channel/future -> owner/receiver -> RPC/I/O/callback -> release ``` Cancellation, close, abort, timeout, health-check, and rejection paths must be independent of the work they control. If such a path acquires a lock held across downstream work, sends into that work's queue, or synchronously waits for its cleanup, it inherits the downstream termination condition. Trace to the local edge that guarantees progress; a caller context alone is insufficient if the blocked primitive does not observe it. For restart/retry/pooling, include generation edges: old callbacks and handles must terminate before reuse, and new state must remain unpublished until all admission gates are ready. ### G3 Detail: Retained-Live-Memory Check For any variable `V` that exhibits growth (`append`, `map[K]=V`, channel send, allocation): 1. Identify which object retains each appended/allocated value and for how long. 2. Locate every applicable bound or release: lexical scope/GC reachability, `clear`/`delete`, truncation, capacity/backpressure, eviction, ownership transfer, generation replacement, and terminal cleanup. 3. Verify at least one bound/release is always reached for every growth path, or prove a finite external admission bound. A literal `= nil` is only one form. Common false positive: only searching assignments misses cleanup through aliases, container deletion, scope exit, eviction, or a transferred owner. Trace retained reachability and the actual lifecycle before filing a Q3 bug. --- ## Appendix A: Module Lifecycle Hypotheses These rows are navigation hints, not proof. Revalidate every API signature, owner, timeout, and bound against the exact checked-out commit before using it in a finding; release branches may differ from current `main`. ### A.1 Proxy / Execution Dispatch Layer | Resource Creation | Destruction Path | Wait Point | Release Event | Accumulation | Bounded By | |-------------------|-----------------|------------|---------------|--------------|------------| | `go handleConnection()` | `defer cc.Close()` | `handleOneMessage()` reading from client | `io.EOF` / `ctx.Done()` / `err` | connection count | `maxConnections` | | `go pipe.kickoff()` (×2) | `src.Shutdown()` → `io.EOF` | `cond.Wait()` in pause | cancel watcher goroutine + timer | `cnTunnels` map | CN count × per-CN connection count | | placeholder tunnel | `selectOneFailed()` | `selectOne()` selecting CN | immediate return | — | — | | transfer goroutine | `defer finishTransfer()` | `pause()` cond.Wait | `defaultTransferTimeout=10s` | — | — | ### A.2 morpc / RPC Transport Layer | Resource Creation | Destruction Path | Wait Point | Release Event | Accumulation | Bounded By | |-------------------|-----------------|------------|---------------|--------------|------------| | `readLoop` goroutine | `conn.Read()` returns error / `readTimeout` triggers ctx check | `conn.Read(readOptions)` | **readTimeout > 0** → periodic return | backend pool | `maxConnections` | | `writeLoop` goroutine | `defer closeConn(false)` | `writeLoop` ctx | ctx.Done() | — | — | | backend connection | GC manager (idle check + inactive check) | — | — | idle backend | `maxIdleDuration` | | Future | immediately after acquisition, install exactly-once `defer f.Close()` before the at-most-once `f.Get()` | `f.Get()` | send/context/response completion; deferred `Close()` covers every return/error path | — | — | ### A.3 Compile / Remote Execution Layer | Resource Creation | Destruction Path | Wait Point | Release Event | Accumulation | Bounded By | |-------------------|-----------------|------------|---------------|--------------|------------| | sender goroutines | `sender.close()` in `RemoteRun` defer | `sendPipeline` each send | caller ctx | — | — | | `messageSenderOnClient` goroutine | `<-receiver.connectionCtx.Done()` | stream.Get() | ctx / stream close | — | — | | stream sender | `Close(true)` → pool return + gauge dec | `waitingTheStopResponse()` | `30s timeout` | stream pool | unknown/admission-dependent; `sync.Pool` is not a capacity bound—revalidate current source | | `messageReceiverOnServer` goroutine | `<-receiver.connectionCtx.Done()` | `NotifyDispatch` | `connectionCtx.Done()` + `dispatchProc.Ctx.Done()` | — | — | | dispatch goroutines | `proc.Ctx.Done()` / channel close | channel send (non-blocking) | non-blocking select + default fallback | channel buffer | `ChannelBufferSize` | ### A.4 Pipeline / Computation Execution Layer | Resource Creation | Destruction Path | Wait Point | Release Event | Accumulation | Bounded By | |-------------------|-----------------|------------|---------------|--------------|------------| | `Pipeline` scope | `defer p.Cleanup(s.Proc, err != nil, isPrepare, err)` | merge (non-blocking) | — | batches | `bat.Clean(mp)` per-batch release |