fix: Addressed race condition in workdir_handler.py#3844
Merged
johanneskoester merged 1 commit intosnakemake:mainfrom Nov 27, 2025
matthewakram:patch-1
Merged
fix: Addressed race condition in workdir_handler.py#3844johanneskoester merged 1 commit intosnakemake:mainfrom matthewakram:patch-1
johanneskoester merged 1 commit intosnakemake:mainfrom
matthewakram:patch-1
Conversation
Race condition existed in workdir_handler.py when for some reason (such as testing suites that might run in parallel) snakemake is run in parallel, and change_to is called, an exception is called, though control-flow can continue as normal without any issues.
Contributor
📝 WalkthroughWalkthroughA single method in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
johanneskoester
approved these changes
Nov 27, 2025
Contributor
|
Thanks a lot! |
johanneskoester
pushed a commit
that referenced
this pull request
Nov 27, 2025
🤖 I have created a release *beep* *boop* --- ## [9.14.0](v9.13.7...v9.14.0) (2025-11-27) ### Features * Support Hy in script directive ([#3824](#3824)) ([2329c9e](2329c9e)) ### Bug Fixes * Add support for pathlib in `notebook` field ([#3811](#3811)) ([7b2180a](7b2180a)) * Addressed race condition in workdir_handler.py ([#3844](#3844)) ([8dbfcfb](8dbfcfb)) * cleanup update-marked output files of failed jobs if there was no backup to restore them ([#3843](#3843)) ([41f1ce8](41f1ce8)) * correct Windows callable path handling ([#3832](#3832)) ([5caad70](5caad70)) * expand env vars on resources ([#3823](#3823)) ([fcfa1bc](fcfa1bc)) * fix backup for output marked by `update` ([#3839](#3839)) ([09c64b7](09c64b7)) * Minor fixes/additions to logging module. ([#3802](#3802)) ([3b3986d](3b3986d)) * mount local storage prefix into containers ([#3840](#3840)) ([f1e8b62](f1e8b62)) * properly format input/output files in case of missing rule to produce them ([#3849](#3849)) ([69d5d24](69d5d24)) * Unpack AnnotatedString in _apply_wildcards ([#3798](#3798)) ([7886508](7886508)) ### Performance Improvements * retrieve storage inputs immediately before scheduling jobs instead of before running the entire workflow ([#3850](#3850)) ([4ac6cda](4ac6cda)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
kjohnsen
pushed a commit
to kjohnsen/snakemake
that referenced
this pull request
Dec 15, 2025
Race condition existed in workdir_handler.py when for some reason (such
as testing suites that might run in parallel) snakemake is run in
parallel, and change_to is called, an exception is called, though
control-flow can continue as normal without any issues.
Here is a more thorough description of how the race condition can occur
For reference, take a look at mkdir from pathlib
```python
def mkdir(self, mode=0o777, parents=False, exist_ok=False):
"""
Create a new directory at this given path.
"""
try:
os.mkdir(self, mode)
except FileNotFoundError:
if not parents or self.parent == self:
raise
self.parent.mkdir(parents=True, exist_ok=True)
self.mkdir(mode, parents=False, exist_ok=exist_ok)
except OSError:
# Cannot rely on checking for EEXIST, since the operating system
# could give priority to other errors like EACCES or EROFS
if not exist_ok or not self.is_dir():
raise
```
Take two threads that are run on snakemake, and both try to cd into
"/fake/address"
Thread 1 and 2:
checks if "/fake/address" (returns False)
calls self.workdir.mkdir(parents=True) (with exists_ok defaulting to
false)
inside the try block: tries to mkdir "/fake/address" which fails with
FileNotFoundError because "/fake" does not exist
catches exception
Thread 1:
calls `self.parent.mkdir(parents=True, exist_ok=True)`
successfully creates "/fake"
Thread 2:
calls `self.parent.mkdir(parents=True, exist_ok=True)`
"/fake" already exists, but that is not a problem
calls `self.mkdir(mode, parents=False, exist_ok=exist_ok)` to create
"/fake/address"
succeeds
Thread 1:
calls `self.mkdir(mode, parents=False, exist_ok=exist_ok)` to create
"/fake/address"
fails because "/fake/address" exists
Whereby there is no problem here that "/fake/address" exists.
Not sure if I should write a test for this, I personally wouldn't even
know how to begin testing this issue, but it seems to have already been
solved by pathlib, so just adding this small param should fix this niche
issue.
This PR does not change intended behavior.
### QC
<!-- Make sure that you can tick the boxes below. -->
* [x] The PR contains a test case for the changes or the changes are
already covered by an existing test case.
* [x] The documentation (`docs/`) is updated to reflect the changes or
this is not necessary (e.g. if the change does neither modify the
language nor the behavior or functionalities of Snakemake).
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
# Release Notes
* **Bug Fixes**
* Enhanced working directory handling to gracefully manage scenarios
where directories already exist, improving reliability during concurrent
operations and preventing unnecessary errors.
<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
kjohnsen
pushed a commit
to kjohnsen/snakemake
that referenced
this pull request
Dec 15, 2025
🤖 I have created a release *beep* *boop* --- ## [9.14.0](snakemake/snakemake@v9.13.7...v9.14.0) (2025-11-27) ### Features * Support Hy in script directive ([snakemake#3824](snakemake#3824)) ([2329c9e](snakemake@2329c9e)) ### Bug Fixes * Add support for pathlib in `notebook` field ([snakemake#3811](snakemake#3811)) ([7b2180a](snakemake@7b2180a)) * Addressed race condition in workdir_handler.py ([snakemake#3844](snakemake#3844)) ([8dbfcfb](snakemake@8dbfcfb)) * cleanup update-marked output files of failed jobs if there was no backup to restore them ([snakemake#3843](snakemake#3843)) ([41f1ce8](snakemake@41f1ce8)) * correct Windows callable path handling ([snakemake#3832](snakemake#3832)) ([5caad70](snakemake@5caad70)) * expand env vars on resources ([snakemake#3823](snakemake#3823)) ([fcfa1bc](snakemake@fcfa1bc)) * fix backup for output marked by `update` ([snakemake#3839](snakemake#3839)) ([09c64b7](snakemake@09c64b7)) * Minor fixes/additions to logging module. ([snakemake#3802](snakemake#3802)) ([3b3986d](snakemake@3b3986d)) * mount local storage prefix into containers ([snakemake#3840](snakemake#3840)) ([f1e8b62](snakemake@f1e8b62)) * properly format input/output files in case of missing rule to produce them ([snakemake#3849](snakemake#3849)) ([69d5d24](snakemake@69d5d24)) * Unpack AnnotatedString in _apply_wildcards ([snakemake#3798](snakemake#3798)) ([7886508](snakemake@7886508)) ### Performance Improvements * retrieve storage inputs immediately before scheduling jobs instead of before running the entire workflow ([snakemake#3850](snakemake#3850)) ([4ac6cda](snakemake@4ac6cda)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Race condition existed in workdir_handler.py when for some reason (such as testing suites that might run in parallel) snakemake is run in parallel, and change_to is called, an exception is called, though control-flow can continue as normal without any issues.
Here is a more thorough description of how the race condition can occur
For reference, take a look at mkdir from pathlib
Take two threads that are run on snakemake, and both try to cd into "/fake/address"
Thread 1 and 2:
checks if "/fake/address" (returns False)
calls self.workdir.mkdir(parents=True) (with exists_ok defaulting to false)
inside the try block: tries to mkdir "/fake/address" which fails with FileNotFoundError because "/fake" does not exist
catches exception
Thread 1:
calls
self.parent.mkdir(parents=True, exist_ok=True)successfully creates "/fake"
Thread 2:
calls
self.parent.mkdir(parents=True, exist_ok=True)"/fake" already exists, but that is not a problem
calls
self.mkdir(mode, parents=False, exist_ok=exist_ok)to create "/fake/address"succeeds
Thread 1:
calls
self.mkdir(mode, parents=False, exist_ok=exist_ok)to create "/fake/address"fails because "/fake/address" exists
Whereby there is no problem here that "/fake/address" exists.
Not sure if I should write a test for this, I personally wouldn't even know how to begin testing this issue, but it seems to have already been solved by pathlib, so just adding this small param should fix this niche issue.
This PR does not change intended behavior.
QC
docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.