Skip to content

Fix intermittent server boot failure caused by working directory race - #727

Merged
soutaro merged 3 commits into
mainfrom
fix-runner-client-boot-race
Aug 7, 2026
Merged

Fix intermittent server boot failure caused by working directory race#727
soutaro merged 3 commits into
mainfrom
fix-runner-client-boot-race

Conversation

@soutaro

@soutaro soutaro commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Motivation

CI hangs intermittently on Ruby 4.0.6 (first seen in #724 / #725) — jobs time out after 25 minutes with the test suite stuck mid-run. The cause is a working-directory race in RunnerClient.create_client, not setup-ruby or gem updates.

create_client checks File.exist?("bin/rails") relative to the process working directory, and spawns the rails runner server with the inherited cwd. But the working directory is process-global state that other threads mutate: RuboCop's config loader evaluates configs with Dir.chdir(config dir) { ERB.new(...).result } while resolving inherit_gem. When the Ruby LSP RuboCop addon activates concurrently with the Rails addon boot thread, the boot thread can observe the rubocop-shopify gem directory as cwd. The bin/rails check then fails and create_client silently falls back to a NullClient.

The race is long-standing but almost never fired until Ruby 4.0.6, whose thread-scheduler changes (Bug #21685 backport) altered thread interleaving enough to make it fire several times per 100 boots on CI runners. That's why bumping setup-ruby past 1.317.0 (which resolves "4.0" to 4.0.6 instead of 4.0.5) broke CI. Trace from a failing boot captured in the investigation PR (#726, now closed):

[RuboCop] Activating RuboCop LSP addon 1.88.2.
Ruby LSP Rails create_client start (cwd=.../vendor/bundle/ruby/4.0.0/gems/rubocop-shopify-2.18.0)
Ruby LSP Rails create_client: bin/rails NOT found (cwd=.../gems/rubocop-shopify-2.18.0)
[RuboCop] Initialized RuboCop LSP addon 1.88.2.

Note that the hang itself only happens in this repo's test suite: several tests busy-wait with sleep(0.1) while ...NullClient for the real client to boot, and the loop never terminates when boot fails because NullClient is the final state. In real editor usage a boot failure does not hang anything — the add-on falls back to the NullClient, logs a warning, and Rails-dependent features are silently unavailable.

Implementation

  • Resolve bin/rails against global_state.workspace_path instead of the cwd, and spawn the server with an explicit chdir: to the workspace path, so neither depends on racy process-global state.
  • Fix the tests' unbounded busy-wait as well: add Addon#join_boot_thread so the boot outcome can be awaited deterministically, and make the tests join the boot thread and assert that the client actually booted. A boot failure now fails the test immediately with a clear message instead of hanging the suite until the CI timeout.

@soutaro
soutaro force-pushed the fix-runner-client-boot-race branch from afd05bb to 8510c69 Compare August 5, 2026 04:25
RunnerClient.create_client checked for bin/rails relative to the current
working directory and spawned the server child process with the inherited
working directory. The working directory is process global state and other
threads may change it concurrently: RuboCop's config loader temporarily
switches to a gem's directory while evaluating configurations inherited
with inherit_gem. When the Ruby LSP RuboCop add-on activation overlaps the
Rails add-on's boot thread, the boot thread can observe the gem's directory
as the working directory, fail the bin/rails check, and silently fall back
to a NullClient. Tests busy-wait for the client to stop being a NullClient,
so the whole suite hung.

Ruby 4.0.6 changed thread context switching (backport of Bug #21685), which
made this long-latent race fire frequently on CI runners.

Resolve bin/rails against the workspace path from the global state instead,
and spawn the server with an explicit chdir to the workspace path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@soutaro
soutaro force-pushed the fix-runner-client-boot-race branch 2 times, most recently from 7f4ef80 to 4f159e4 Compare August 5, 2026 04:41
Several tests busy-waited with `sleep(0.1) while ...NullClient` until the
Rails add-on's boot thread replaced the NullClient with a real client. When
booting fails, the NullClient is the final state, so these loops never
terminate and the whole suite hangs until the CI job times out.

Add Addon#join_boot_thread so the boot outcome can be awaited
deterministically, and make the tests join the boot thread and assert that
the resulting client is not a NullClient, so a boot failure fails the test
immediately with a clear message.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@soutaro
soutaro force-pushed the fix-runner-client-boot-race branch from 4f159e4 to b753e66 Compare August 5, 2026 04:44
@soutaro
soutaro marked this pull request as ready for review August 5, 2026 04:59
@soutaro
soutaro requested a review from a team as a code owner August 5, 2026 04:59
Comment thread lib/ruby_lsp/ruby_lsp_rails/runner_client.rb

@jesse-shopify jesse-shopify left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like seeing the removal of all the sleep calls. This is a much improved design.

@soutaro
soutaro force-pushed the fix-runner-client-boot-race branch from c47b378 to 6b7f6fa Compare August 7, 2026 01:01
Covers both halves of the working directory fix: locating bin/rails and
spawning the server both have to use the workspace path rather than the
process working directory. The test fails against either half being
reverted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@soutaro
soutaro force-pushed the fix-runner-client-boot-race branch from 6b7f6fa to 35edca5 Compare August 7, 2026 01:07
@soutaro
soutaro merged commit 7e526fe into main Aug 7, 2026
61 checks passed
@soutaro
soutaro deleted the fix-runner-client-boot-race branch August 7, 2026 01:24
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.

4 participants