Suppress potential errors when releasing filelocks on unix #25771
Conversation
- closes emscripten-core#24609 - The `WindowsFileLock` class already has a suppression in place around `os.remove`. This commit adds a similar supression for `UnixFileLock` - These errors are typically raised when another instance of `emcc` released the lock file. This is a common pattern in the build systems of large projects. Ex: `ninja -j32`
jspanchu
force-pushed
the
workaround-race-filelock-unix
branch
from
d313296 to
7ef387b
Compare
| with suppress(FileNotFoundError): | ||
| os.unlink(self._lock_file) | ||
| with suppress(OSError): | ||
| fcntl.flock(fd, fcntl.LOCK_UN) |
There was a problem hiding this comment.
Are both of these needed to fix your issue? Or is just one or the other enough?
It seems like neither should be necessary, since if we are holding the lock we should always be able to unlock it, right? It should be impossible for another process acquire or delete the lock file until LOCK_UN, no?
There was a problem hiding this comment.
The os.unlink is what I observed to cause trouble (also seen in the issue linked in PR description). The other one, should not be necessary.
It seems like neither should be necessary, since if we are holding the lock we should always be able to unlock it, right? It should be impossible for another process acquire or delete the lock file until LOCK_UN, no?
I would think so, but in practice, os.unlink gets executed after someone else deletes it. Could it be an issue with the port, and not the filelock.py?
There was a problem hiding this comment.
I would think so, but in practice,
os.unlinkgets executed after someone else deletes it.
Im struggling to see how anyone else could be deleting this file. But clearly it is happening, I just want to know how / why
|
We do a lot of building with of all our library with a lot of parallelism, but I don't think i've run into this issue locally or in our CI. For example, I have 128 cores and often rebuild the library cache from scratch I've not run into this. I will try to build a reproducer. |
|
I wrote a test to try an repro this but so far its not reproducing: #25772 |
2 participants
WindowsFileLockclass already has a suppression in place aroundos.remove. This commit adds a similar supression forUnixFileLockemccreleased the lock file. This is a common pattern in the build systems of large projects. Ex:ninja -j32