Skip to content

bpo-43468: Per instance locking for functools.cached_property #27609

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

Closed
wants to merge 16 commits into from

Conversation

rhettinger
Copy link
Contributor

@rhettinger rhettinger commented Aug 4, 2021

Draft PR. It needs a NEWS entry, perhaps more tests, and perhaps some more factoring.

https://bugs.python.org/issue43468

@rhettinger rhettinger added the type-bug An unexpected behavior, bug, or error label Aug 4, 2021
@sweeneyde
Copy link
Member

It looks like threading._register_atexit could lazily import functools to avoid the circular threading <=> functools import

@@ -18,7 +18,7 @@
from collections import namedtuple
# import types, weakref # Deferred to single_dispatch()
from reprlib import recursive_repr
from _thread import RLock
from threading import RLock, Condition, get_ident
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces an import cycle which causes tests to fail.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a race condition on 980-983.
Scenario:

  • 3 threads, 1 registered in updaters
  • 2nd and 3rd will wait
  • both waiting threads will see registration of registered thread disappear
    Mutual exclusion breaks.

@Erotemic
Copy link
Contributor

Erotemic commented Aug 31, 2021

Can someone confirm or deny if my solution on twitter works?

I also made a PR here: rhettinger#7
I suppose one test is to see if the dashboards pass.

@ghost
Copy link

ghost commented Apr 16, 2022

The following commit authors need to sign the Contributor License Agreement:

Click the button to sign:
CLA not signed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review DO-NOT-MERGE type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants