Skip to content

fix(forge): monitor beam rewriting worker process#370

Merged
doorgan merged 1 commit into
expert-lsp:mainfrom
QuinnWilton:fix/forge-beams-unlinked-spawn
Jun 24, 2026
Merged

fix(forge): monitor beam rewriting worker process#370
doorgan merged 1 commit into
expert-lsp:mainfrom
QuinnWilton:fix/forge-beams-unlinked-spawn

Conversation

@QuinnWilton

@QuinnWilton QuinnWilton commented Feb 10, 2026

Copy link
Copy Markdown
Contributor

This is part of a set of 4 PRs that arose out of some static analysis tooling I'm working on:

That means that these aren't crashes or issues that I've observed in practice, however based on my reading of the code, they do represent issues worth addressing.

Problem:

Beams.apply_to_all/2 spawns its worker process using a bare spawn/1, and then blocks in block_until_done/3 until it receives a :progress message from it. If the worker crashes, this message will never arrive, and the caller will block indefinitely.

Solution

Replace spawn/1 with spawn_monitor/1, and add a receive clause in block_until_done/3 to handle the worker unexpectedly terminating before a :progress message is sent, so that we're able to immediately raise instead of blocking forever.

In the successful case, the monitor is flushed to avoid :DOWN messages building up and causing a mailbox leak.

Replace bare spawn with spawn_monitor in Beams.apply_to_all/2.
If the worker process crashed, the parent would hang forever
waiting for :progress messages that would never arrive.

Propagate the monitor ref through block_until_done and raise on
{:DOWN, ...} so crashes surface immediately. Demonitor on normal
completion.
@doorgan doorgan force-pushed the fix/forge-beams-unlinked-spawn branch from eb80bc2 to b81abb3 Compare June 24, 2026 19:03
@doorgan doorgan merged commit c8be4e9 into expert-lsp:main Jun 24, 2026
49 checks passed
@expert-backports

Copy link
Copy Markdown

Successfully created backport PR for v0.1:

expert-backports Bot added a commit that referenced this pull request Jun 24, 2026
# Description
Backport of #370 to `v0.1`.

Co-authored-by: Quinn Wilton <quinn@quinnwilton.com>
doorgan pushed a commit that referenced this pull request Jun 24, 2026
This is part of a set of 4 PRs that arose out of some static analysis
tooling I'm working on:
- #369
- #370
- #371
- #372

That means that these aren't crashes or issues that I've observed in
practice, however based on my reading of the code, they do represent
issues worth addressing.

## Problem:
This is effectively the same issue as in
#370, where the use of a bare
`spawn/2` causes the spawning process to hang indefinitely while waiting
for a message from the spawned process, and preventing `Credo`
diagnostics from being updated.

## Solution
Replace `spawn/1` with `spawn_monitor/1`, and add a `receive` clause to
handle the spawned process unexpectedly terminating before sending a
result.

In the successful case, the monitor is flushed to avoid `:DOWN` messages
building up and causing a mailbox leak.

A 30 second timeout is also added, to prevent buggy or broken `Credo`
modules from causing the entire system to hang.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants