trbs / pid

Pidfile featuring stale detection and file-locking, can also be used as context-manager or decorator
https://pypi.python.org/pypi/pid/
Apache License 2.0
102 stars 26 forks source link

Double close #22

Closed humanthrope closed 5 years ago

humanthrope commented 5 years ago

The PidFile class registers close() to be called at process exit time, but if the user manually calls close(), this can lead to a race condition where Instance A of a script closes the pid file and instance Instance B of the same script is allowed to run, but A delete's B's pid file.

Pseudo Code

pid_file = PidFile(...)
try:
    pid_file.create()
except Exception:
    if not pid_file.filename or not os.path.exists(pid_file.filename):
        raise
    raise MyAlreadyRunningError

try:
    do_something()
finally:
    pid_file.close()
    # A hasn't called at exit yet, but B is allowed to run
    # Shortly later, A exits and A's `atexit` closes the pid file that B creates

It would be possible to not call close(), but we have tests to check for MyAlreadyRunningError and pytest does not exit (therefore never calls the registered atexit handlers) until after the full suite of test has been run.

trbs commented 5 years ago

I see... good catch !

Could you please double check if aad0c9c02d416f08a18a25cf6c77ad1b2e2c8b53 properly solves this ?

humanthrope commented 5 years ago

Looks good to me. Thanks!

trbs commented 5 years ago

Awesome, thanks for reporting the issue.