Skip to content

Serialize the frpc download so parallel harbor rollouts can share a fresh machine - #1568

Open
sergiopaniego wants to merge 1 commit into
openenv:mainfrom
sergiopaniego:fix-frpc-download-race
Open

sergiopaniego wants to merge 1 commit into
openenv:mainfrom
sergiopaniego:fix-frpc-download-race

Conversation

@sergiopaniego

@sergiopaniego sergiopaniego commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Starting several openenv harbor rollout --expose gradio processes at once on a fresh machine made some of them fail their first task with Text file busy, because each one's first tunnel downloads gradio's frpc binary to the same path. The download now happens under a file lock, so one process downloads and the others wait.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • New environment
  • Refactoring

What was wrong

Running 3 to 12 rollout processes in parallel on one HF Job, each on its own --port, some failed their first task before the agent ran, with Text file busy. Starting the workers 20 s apart avoided it, and once the binary was on disk every later tunnel worked.

Root cause

GradioForwarder.start (src/openenv/core/harness/capture/forwarding.py) calls gradio's setup_tunnel, which calls Tunnel.download_binary (gradio/tunneling.py:83-110, gradio 6.31.0). It checks whether BINARY_PATH exists and then writes the binary in place at $HF_HOME/gradio/frpc/frpc_<os>_<arch>_v0.3. While the first process is still writing, a second sees the file, skips the download and executes it: the kernel refuses to exec a file open for writing (ETXTBSY), or the process runs a truncated binary.

Fix

GradioForwarder._ensure_frpc_binary takes an exclusive fcntl.flock on <binary>.lock next to the binary and calls gradio's own Tunnel.download_binary inside it. The first process downloads, the others wait and then find a complete file. setup_tunnel runs as before and finds the binary in place. Download errors still raise ForwardingError, the lock costs nothing once the binary exists, and Windows (no fcntl) keeps gradio's behaviour. No gradio internals are patched and no dependency is added.

RFC Status

  • Not required (bug fix, docs, minor refactoring)

Test Plan

  • New tests/envs/test_harbor_frpc_download_race.py: two threads call _ensure_frpc_binary against a fake downloader shaped like gradio's (check, then write in place, pausing halfway). Without the lock the second caller reads the half-written file and the test fails. With it, the second caller reads the full file and only one download happens.
  • tests/envs/test_harbor_forwarding_lifecycle.py: the fake gradio.tunneling module also provides BINARY_PATH and Tunnel.

Across tests/envs/test_harbor_*.py the only failure (test_harbor_capture_server.py::test_start_refuses_a_port_held_by_a_non_capture_listener) also fails on main. ruff format, ruff check and usort are clean.

Claude Code Review

N/A


Note

Low Risk
Targets gradio tunnel startup with an advisory file lock and unchanged Windows path; adds regression coverage with limited blast radius beyond first tunnel setup.

Overview
Parallel harbor rollouts no longer race on a half-written frpc binary. GradioForwarder.start now calls _ensure_frpc_binary, which opens a .lock file beside gradio’s binary path, takes an exclusive fcntl.flock when available, then invokes gradio’s Tunnel.download_binary. The first process downloads; others wait until the lock releases, then reuse the complete file. fcntl is optional (Windows import failure leaves fcntl unset so locking is skipped and behavior matches gradio’s today).

Tests stub BINARY_PATH and Tunnel.download_binary in the existing lifecycle test so start still works. A new test_harbor_frpc_download_race runs two threads against a fake downloader that pauses mid-write and asserts the second thread only sees a fully written binary and that only one download runs.

Reviewed by Cursor Bugbot for commit ddf2971. Bugbot is set up for automated code reviews on this repo. Configure here.

This branch has not been deployed

No deployments
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