ReviewOS

home-lang/home

fix(program): make source replacement atomic and audit redirect borrows

#472
Closed chris opened this 8 days ago ·
8 days ago

Source-level audit while investigating #457 found two pre-existing lifetime risks in Program.updateSource (packages/ts_program/src/ts_program.zig, currently around lines 317-336). It destroys cached compilation before allocating, duplicates the new source, removes/frees the old source-map key/value, then allocates a replacement key and reinserts. Failure after removal can leave File.source referring to freed bytes and leak the new duplicate. Also, addRedirectFile borrows target.source, but successful replacement of the canonical source does not refresh redirect source slices. This is an audit finding, not yet a dedicated failing-allocator/runtime reproduction.

Acceptance: add precise allocation-failure/recovery/leak regressions for source replacement; publish the new source/map/compilation state atomically; verify canonical and redirected-file ownership/lifetimes after successful updates; cover direct and redirected updates and preserve package-id semantics. Keep source snapshot invalidation consistent. The current performance work uses transient per-collection facts and continues excluding redirected files; it does not claim to fix these underlying update semantics. Track under #416.

8 days ago

The performance change d330198 deliberately scopes source-marker snapshots to each compilation pass and excludes redirect sources; it does not repair the updateSource ownership/atomicity risks identified here. Its replacement/redirect tests check snapshot invalidation and exclusion, not failure recovery of updateSource itself. Those dedicated reproductions and fixes remain required. Results and this limitation are documented in 20084a4.

8 days ago

Resolved by dc88471, pushed directly to main. Source replacement now allocates before publishing, preserves the existing source-map key, resolves redirect updates to the canonical file, refreshes every sibling redirect, and invalidates compilation, owner, imports, and marker state only after allocation succeeds. Incremental recompilation canonicalizes and deduplicates redirect and canonical paths. Permanent regressions force allocation failure and verify full rollback and recovery, then update through a real package-ID redirect and require one canonical recompile while preserving package identity. Verification: ts_program 225/225; final guarded peak 2,027 MB with 35% host low-water; Zig format, diff checks, and Pickier passed. The broader ts_lsp build was not claimed green: the memory guard stopped it at 3,610 MB under the 3,584 MB ceiling, and the ceiling was not raised.

Sign in to comment on this issue.