v8.35

[jkool702]

Summary by Sourcery

Fix worker spawn deadlock in the ring scanner and extend benchmarking coverage for additional pipeline patterns.

Bug Fixes:

• Prevent deadlock in the ring scanner by replacing buffered dprintf-based worker spawn signaling with unbuffered snprintf+write to the spawn pipe.

Tests:

• Extend run_benchmark.bash to benchmark frun both with direct input redirection and via cat-piped input in combination with wc -l.

Chores:

• Bump forkrun_ring loadable version metadata from v8.34 to v8.35 and perform minor code/comment cleanups.

[sourcery-ai]

Reviewer's Guide

Revises forkrun ring scanner worker-spawn signaling to use unbuffered writes instead of dprintf to fix a deadlock, keeps v8.34 implementation as a frozen reference, and extends the benchmark script to cover additional pipeline invocation patterns.

Sequence diagram for unbuffered worker-spawn signaling in ring_scanner

sequenceDiagram
    actor User
    participant BashLoop as bash_frun_driver
    participant Scanner as ring_scanner_main
    participant SpawnPipe as fd_spawn
    participant Worker as worker_process

    User->>BashLoop: run frun ...
    BashLoop->>Scanner: start ring_scanner_main(fd, fd_spawn)

    loop scanning_batches
        Scanner->>Scanner: compute_needed_workers()
        alt need_initial_workers
            Scanner->>SpawnPipe: write("N\n") using write
            SpawnPipe->>BashLoop: data "N\n" available
            BashLoop->>BashLoop: read N from fd_spawn
            BashLoop->>Worker: fork/exec N workers
            Worker-->>Scanner: start consuming ring batches
        else adjust_workers_dynamically
            Scanner->>SpawnPipe: write("grow\n") using write
            SpawnPipe->>BashLoop: data "grow\n" available
            BashLoop->>BashLoop: read grow from fd_spawn
            BashLoop->>Worker: fork/exec grow additional workers
            Worker-->>Scanner: consume additional batches
        end
    end

    Scanner-->>BashLoop: scanner_finished flag set
    BashLoop-->>User: pipeline completes without deadlock

Loading

File-Level Changes

Change	Details	Files
Fix scanner worker-spawn signaling deadlock by replacing buffered dprintf with snprintf+write on the spawn FD.	In ring_scanner_main, replace all dprintf(fd_spawn, "%lu\n", ...) calls with snprintf into a small stack buffer followed by a direct write() guarded by SYS_CHK. Apply this unbuffered write pattern in all three places that request additional workers: initial ramp-up, steady-state tuning, and tail-phase top-up. Add comments marking this as a critical fix to avoid scanner-side buffering that could stall the Bash loop waiting for data that never flushes.	ring_loadables/forkrun_ring.c
Preserve the previous v8.34 implementation as a versioned snapshot with only trivial whitespace/style normalization.	Keep ring_loadables/v8.34/forkrun_ring.c as a frozen copy of the v8.34 logic so regressions can be bisected against it. Normalize a few stray trailing spaces and formatting inconsistencies without changing behavior.	ring_loadables/v8.34/forkrun_ring.c
Broaden benchmarking coverage to include cat	frun pipelines in addition to direct redirection runs.	In run_benchmark.bash, add two new timing cases that pipe input via cat into frun, once discarding output and once counting lines with wc -l. Retain the existing three benchmark patterns and loop structure.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The new snprintf+write sequences for communicating spawn counts are duplicated in several places in ring_scanner_main; consider factoring this into a small helper (e.g., ring_write_u64_line(fd, value)) to centralize formatting and error handling.
• When using write() to send the formatted spawn counts, only a single write call is made; if you want this to be robust beyond the "very short to a pipe" case, consider looping until all bytes are written or returning a clear error on short writes.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new snprintf+write sequences for communicating spawn counts are duplicated in several places in ring_scanner_main; consider factoring this into a small helper (e.g., ring_write_u64_line(fd, value)) to centralize formatting and error handling.
- When using write() to send the formatted spawn counts, only a single write call is made; if you want this to be robust beyond the "very short to a pipe" case, consider looping until all bytes are written or returning a clear error on short writes.

## Individual Comments

### Comment 1
<location> `ring_loadables/forkrun_ring.c:695-698` </location>
<code_context>
                                 if (n_spawn > (W_max_val - W)) n_spawn = W_max_val - W;
                                 if (fd_spawn >= 0) {
-                                    dprintf(fd_spawn, "%lu\n", n_spawn);
+                                    // CRITICAL FIX: Replace buffered dprintf with unbuffered write
+                                    char sbuf[64];
+                                    int slen = snprintf(sbuf, sizeof(sbuf), "%lu\n", n_spawn);
+                                    if (slen > 0) SYS_CHK(write(fd_spawn, sbuf, slen));
+                                    
                                     W += n_spawn;
</code_context>

<issue_to_address>
**issue (bug_risk):** Guard against snprintf returning a length larger than the buffer to avoid over-reading in write()

Because `snprintf` returns the number of characters that *would* have been written, `slen` can be larger than `sizeof(sbuf) - 1` if truncation occurs, even though the buffer is smaller. Using `slen` directly in `write()` can therefore read past `sbuf`.

Clamp the length passed to `write()`, for example:

```c
int slen = snprintf(sbuf, sizeof(sbuf), "%lu\n", n_spawn);
if (slen > 0) {
    size_t to_write = (slen < (int)sizeof(sbuf)) ? (size_t)slen : sizeof(sbuf) - 1;
    SYS_CHK(write(fd_spawn, sbuf, to_write));
}
```

The same clamping logic should be applied at the other `fd_spawn` write sites below.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): Guard against snprintf returning a length larger than the buffer to avoid over-reading in write()

Because snprintf returns the number of characters that would have been written, slen can be larger than sizeof(sbuf) - 1 if truncation occurs, even though the buffer is smaller. Using slen directly in write() can therefore read past sbuf.

Clamp the length passed to write(), for example:

int slen = snprintf(sbuf, sizeof(sbuf), "%lu\n", n_spawn);
if (slen > 0) {
    size_t to_write = (slen < (int)sizeof(sbuf)) ? (size_t)slen : sizeof(sbuf) - 1;
    SYS_CHK(write(fd_spawn, sbuf, to_write));
}

The same clamping logic should be applied at the other fd_spawn write sites below.

[jkool702]

/gemini review

[sonarqubecloud]

Quality Gate failed

Failed conditions
177 Security Hotspots
69.1% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE