Skip to content

fix: Addressed race condition in workdir_handler.py#3844

Merged
johanneskoester merged 1 commit intosnakemake:mainfrom
matthewakram:patch-1
Nov 27, 2025
Merged

fix: Addressed race condition in workdir_handler.py#3844
johanneskoester merged 1 commit intosnakemake:mainfrom
matthewakram:patch-1

Conversation

@matthewakram
Copy link
Copy Markdown
Contributor

@matthewakram matthewakram commented Nov 21, 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

    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

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • 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).

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.

✏️ Tip: You can customize this high-level summary in your review settings.

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.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 21, 2025

📝 Walkthrough

Walkthrough

A single method in WorkdirHandler is updated to use exist_ok=True when creating directories, preventing errors when the target directory already exists and improving race-condition safety without altering the overall control flow.

Changes

Cohort / File(s) Summary
WorkdirHandler directory creation safety
src/snakemake/common/workdir_handler.py
Updated change_to method to use exist_ok=True in directory creation, allowing graceful handling of pre-existing directories and improving race-condition safety.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single method modified with a straightforward parameter change to exist_ok=True
  • No changes to method signature or public API
  • Behavior remains semantically consistent with only error-handling improvement

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses the main change: fixing a race condition in workdir_handler.py, which matches the primary objective and code change.
Description check ✅ Passed The PR description thoroughly explains the race condition issue with clear examples and references to pathlib's solution, addressing both the problem and the fix.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@matthewakram matthewakram changed the title Addressed race condition in workdir_handler.py fix: Addressed race condition in workdir_handler.py Nov 21, 2025
@johanneskoester johanneskoester enabled auto-merge (squash) November 27, 2025 12:33
@johanneskoester
Copy link
Copy Markdown
Contributor

Thanks a lot!

@johanneskoester johanneskoester merged commit 8dbfcfb into snakemake:main Nov 27, 2025
61 of 66 checks passed
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).
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.

2 participants