fix(init): Correctly lock run directories #209
Conversation
`sentry_shutdown` now cleans up after itself, and the code is covered by integration tests anyway.
There was a problem hiding this comment.
Disclaimer: Only looked at the UNIX implementation so far. I think this implementation is inherently racy and can only be solved by locking first. I'm not sure if you're flocking the file, you even need to check the PID?
For reference, here's an implementation in Python based on the C version:
https://github.com/benediktschmitt/py-filelock/blob/b30bdc4fb998f5f4350257235eb51147f9e81862/filelock.py#L377
And the corresponding stack overflow question:
https://stackoverflow.com/questions/17708885/flock-removing-locked-file-without-race-condition
jan-auer
changed the title
|
Same with windows, you don't need to write the PID, just make a call to the _locking function https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/locking?view=vs-2019 The example Jan linked is great and has a windows version too |
src/path/sentry_path_unix.c
Outdated
| { | ||
| lock->is_locked = false; | ||
|
|
||
| int fd = open(lock->path->path, O_RDWR | O_CREAT | O_TRUNC, |
There was a problem hiding this comment.
You might want to pass O_EXCL here. Since unlock deletes the file first, this should be safe. Also, since you're not writing to the file, would it be sufficient to open with O_RDONLY?
There was a problem hiding this comment.
I think its both simpler and easier to understand without O_EXCL. The problem is with leftover files that were not correctly cleaned up. Like in the case of a crash.
There was a problem hiding this comment.
Good point, in case of a crash, we might consider to actually clean those up, wdyt?
This allows multi-process usage, and also fixes file deletion on windows.
fixes #220
fixes #223