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

Don't initialize BaseFileLock when just returning existing instance #334

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

ethanbb
Copy link
Contributor

@ethanbb ethanbb commented Jun 11, 2024

Fixes #331. Avoids re-initializing a FileLock or BaseFileLock if is_singleton is true and __new__ just returns the singleton, which allows the lock to be re-acquired via the constructor if it has already been acquired without resetting the count.

@gaborbernat gaborbernat merged commit c64787f into tox-dev:main Jun 11, 2024
31 checks passed
@ethanbb ethanbb deleted the fix-nested-cm branch June 11, 2024 21:52
@tamird
Copy link

tamird commented Jun 12, 2024

This (via the 3.15.0 release) may be responsible for breaking virtualenv. See pypa/virtualenv#2735.

@tamird
Copy link

tamird commented Jun 12, 2024

Ah this is API breaking (__init__ no longer has the same signature).

@ethanbb
Copy link
Contributor Author

ethanbb commented Jun 12, 2024

OK sorry, can fix - just need to put in a dummy __init__ that takes *args, **kwargs and is a noop?

@tamird
Copy link

tamird commented Jun 12, 2024

Doesn't sound like an awesome idea to me because type safety is out the window. Sounds like a test that subclasses and overrides __init__ would be prudent either way.

@schmod
Copy link

schmod commented Jun 12, 2024

Can somebody yank this release from PyPi? This is an ecosystem-breaking change (doubly so because few folks will have the installation of virtualenv itself behind a lockfile)

Nevermind, I see that's already been done! Many thanks!

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

Successfully merging this pull request may close these issues.

is_singleton=True with nested lock hangs
4 participants