Skip to content

fix(server): avoid holding map iterators across suspension in PCM paths - #489

Merged
16bit-ykiko merged 1 commit into
mainfrom
fix/pcm-dispatch-uaf
Jul 6, 2026
Merged

16bit-ykiko merged 1 commit into
mainfrom
fix/pcm-dispatch-uaf

Conversation

@16bit-ykiko

@16bit-ykiko 16bit-ykiko commented Jul 6, 2026 •

Copy link
Copy Markdown
Member

Two spots in the PCM build paths held references into `workspace.path_to_module` (a `DenseMap`) across coroutine suspension points. While a PCM build is awaited, a concurrent `didSave` can insert into or erase from that map; a rehash then invalidates the held iterator, and the resumed coroutine dereferences freed memory (use-after-free — garbage logs at best, crash at worst). The window is realistic: during a module project's warm-up, PCM builds run back-to-back, and adding a new `.cppm` file at that moment triggers the insert.

  • `init_compile_graph`'s dispatch lambda: took `mod_it` before two awaits (`send_stateless`, commit on the thread pool) and dereferenced it on both success and failure paths afterward. The module name is now copied out once before any suspension; the iterator is never touched again.
  • `ensure_deps`'s buffer-import scan: awaited `compile_deps` inside a range-for over the same map. It happened to be safe today (unconditional `break` right after), but any future edit that continues iterating would reintroduce the bug. The lookup now completes before the await, which operates on a copied id.

No behavior change. The race itself cannot be reproduced deterministically in a test; verified by review that no iterator or reference into the map lives across a suspension point, plus a full local build and unit-test run (843 passed). CI runs the full matrix.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Compiler orchestration in compiler.cpp is refactored so module name/PID values are copied from map lookups into local variables before coroutine suspension points (co_await), and these stable copies are subsequently used for cache checks, logging, and dependency compilation dispatch instead of iterators or references.

Changes

Suspension-safe compiler lookups

Layer / File(s) Summary
PCM dispatch module_name stabilization
src/server/compiler/compiler.cpp
The dispatch lambda copies module_name from the map iterator before any co_await, then uses it for cache hit/miss checks, key construction, bp.module_name assignment, and all subsequent logging (skip, hit, miss, worker failure, commit failure, success).
ensure_deps import scan lookup-before-await
src/server/compiler/compiler.cpp
Import scanning computes module_pid and a found decision from workspace.path_to_module before suspending, skips unknown imports, and only co_await compile_deps(module_pid) conditionally afterward, preventing use of invalidated iterators across the await.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • clice-io/clice#385: Introduces the compile_deps entrypoint that this PR's ensure_deps lookup-before-await change directly calls.
  • clice-io/clice#454: Overlaps with the same init_compile_graph/ensure_deps compiler orchestration paths modified here.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main fix: avoiding map iterators across coroutine suspension in PCM-related paths.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pcm-dispatch-uaf

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@16bit-ykiko
16bit-ykiko merged commit 8f5614b into main Jul 6, 2026
22 checks passed
@16bit-ykiko
16bit-ykiko deleted the fix/pcm-dispatch-uaf branch July 6, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant