Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double close #22

Closed
humanthrope opened this issue Feb 12, 2019 · 3 comments
Closed

Double close #22

humanthrope opened this issue Feb 12, 2019 · 3 comments

Comments

@humanthrope
Copy link

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
Copy link
Owner

trbs commented Feb 14, 2019

I see... good catch !

Could you please double check if aad0c9c properly solves this ?

@humanthrope
Copy link
Author

Looks good to me. Thanks!

@trbs
Copy link
Owner

trbs commented Feb 15, 2019

Awesome, thanks for reporting the issue.

@trbs trbs closed this as completed Feb 15, 2019
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

No branches or pull requests

2 participants