Fixed rare race condition error when creating the basetemp directory #5525
Conversation
nicoddemus
added
the
needs backport
Codecov Report
@@ Coverage Diff @@
## master #5525 +/- ##
=======================================
Coverage 96.11% 96.11%
=======================================
Files 117 117
Lines 25695 25695
Branches 2493 2493
=======================================
Hits 24696 24696
Misses 695 695
Partials 304 304
Continue to review full report at Codecov.
|
| @@ -33,7 +33,7 @@ def ensure_reset_dir(path): | |||
| """ | |||
| if path.exists(): | |||
| rmtree(path, force=True) | |||
| path.mkdir() | |||
There was a problem hiding this comment.
it absolutely has to fail when doing a ensure_reset
There was a problem hiding this comment.
The problem as I understand, is a race condition between workers trying to create the same basedir at the same time; worker 1 and 2 (for example) manage to rmtree at the same time, but only one of them will succeed when they get to path.mkdir()
There was a problem hiding this comment.
ensure_dir_reset is about completely resetting a folder, if the invariant does not hold, something went baldy wrong, and 2 processes that absolutely should not use the same folder will do so
Lines 58 to 60 in 9136b3f
it absolutely never must pass if the observed error happens - basetemp invocation itself is not concurrency save ever
There was a problem hiding this comment.
You are right, my bad, indeed basetemp is created by the master process so there's shouldn't be a race condition there.
I'm closing this and the related issue, I will investigate some more on our side as we do pass a basetemp around. 👍
Thanks for the quick review!
asottile
removed
the
needs backport
Fix #5524
Probably hard/impossible to reproduce in a test, I'm afraid.