Skip to content

Fix a crash from scheduler/worker disconnect [1.6-patch-2] - #2643

Open
cormacrelf wants to merge 2 commits into
TraceMachina:mainfrom
cormacrelf:1.6-patch-2
Open

Fix a crash from scheduler/worker disconnect [1.6-patch-2]#2643
cormacrelf wants to merge 2 commits into
TraceMachina:mainfrom
cormacrelf:1.6-patch-2

Conversation

@cormacrelf

@cormacrelf cormacrelf commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes a bug where a routine scheduler disconnect could panic the worker in some cases if an action happened to be in transit at the time. If you worker and scheduler run in the same instance, this kills the entire nativelink process.

The crash happens because the worker panics it cannot get its actions_in_transit counter to zero within a timeout, however the very act of disconnecting and cancelling the action-in-transit future prevented decrementing the counter and returning it to zero.

This fixes it with RAII so that when the action's future is cancelled (dropped) the counter still gets decremented.

Full crash path

  1. LocalWorkerImpl::run returns Err whenever the scheduler connection drops — an ordinary, expected event that the worker is supposed to recover from by killing its running actions and reconnecting.

  2. Before it can do that, LocalWorker::run waits for actions_in_transit to drain to zero. If it doesn't drain within ACTIONS_IN_TRANSIT_TIMEOUT_S (10s), it bails out instead of reconnecting:

    // nativelink-worker/src/local_worker.rs:922
    error!(ERROR_MSG);
    return Err(err.append(ERROR_MSG));

    This is fine, and the PR does not change this behaviour.

  3. That Err is the return value of the worker's root future, so it comes out of try_join_all(root_futures) in inner_main, which panics on any root future error:

    // src/bin/nativelink.rs:771
    if let Err(e) = try_join_all(root_futures).await {
        panic!("{e:?}");
    }

    inner_main is awaited directly under runtime.block_on on the main thread, so this panic terminates the process — including the scheduler, the CAS, and every other worker in a colocated deployment.

The reason the drain could never complete: actions_in_transit was incremented before spawning the action future, but decremented from inside it, once create_and_add_action resolved:

self.actions_in_transit.fetch_add(1, Ordering::Release);   // before spawn
...
.map(move |r| {
    actions_in_transit.fetch_sub(1, Ordering::Release);    // inside the future
    r
})

The fix introduces ActionsInTransitGuard, an RAII guard that increments the counter on construction and decrements it in Drop, so the count is correct whether the action future completes normally or is aborted mid-flight. The disconnect then drains immediately and the worker reconnects as intended.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

New regression test disconnect_with_action_in_transit_reconnects_test in nativelink-worker/tests/local_worker_test.rs.

Also adjusts the test harness's sleep stub to yield_now() instead of a no-op, so loops that spin on the sleep function still give the runtime a chance to run Drop::drop on aborted futures, matching real scheduler behavior.

Checklist

  • Updated documentation if needed
  • Tests added/amended
  • bazel test //... passes locally
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nativelink Ready Ready Preview Jul 31, 2026 5:41am
nativelink-aidm Ready Ready Preview Jul 31, 2026 5:41am

Request Review

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