Skip to content

Python: fix DependencyWorkspace race on invalid leftover dir#7790

Merged
knutwannheden merged 1 commit into
mainfrom
workspace-lock
May 27, 2026
Merged

Python: fix DependencyWorkspace race on invalid leftover dir#7790
knutwannheden merged 1 commit into
mainfrom
workspace-lock

Conversation

@knutwannheden
Copy link
Copy Markdown
Contributor

Motivation

DependencyWorkspace._create_workspace_with_content crashes with OSError: [Errno 66] Directory not empty when the target cache directory already exists but is invalid (missing .venv or version.txt, typically from a previous crashed build). The old logic tried os.rename(tmp_dir, final_dir) and only handled the race-loss case where another process had built a valid workspace at the destination — if the leftover was invalid, it re-raised.

There are two underlying problems:

  1. macOS rename(2) semantics. On macOS, rename over a non-empty directory returns ENOTEMPTY (errno 66). Linux happens to allow it atomically, but macOS does not. Python's stdlib has no portable atomic-replace-directory primitive (renameat2(RENAME_EXCHANGE) is Linux-only and not exposed).
  2. No serialization between concurrent creators. Two processes building the same cache_key simultaneously could both reach the rename step, with one observing the other's half-built result.

Summary

  • Acquire an fcntl.flock(LOCK_EX) on .{cache_key}.lock for the entire create operation. The lock is held across uv sync — intentional, since only one process should build the same workspace at a time.
  • Inside the lock: re-check _is_valid(final_dir) (another holder may have built it while we waited), then shutil.rmtree any invalid leftover so the subsequent os.rename always targets a non-existent path.
  • Drop the now-unreachable post-rename race-handling block.

Test plan

  • New regression test test_get_or_create_recovers_from_invalid_leftover seeds an invalid final_dir and verifies get_or_create rebuilds rather than crashing.
  • pytest tests/python/template/test_dependency_workspace.py — 18/18 pass.
  • Full pytest tests/python/ — 1441 passed, 6 skipped, 0 failed.

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite May 27, 2026
@knutwannheden knutwannheden merged commit 7d86732 into main May 27, 2026
1 check passed
@knutwannheden knutwannheden deleted the workspace-lock branch May 27, 2026 08:03
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant