Skip to content

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

Closed
rhettinger wants to merge 16 commits intopython:mainfrom
rhettinger:cached_property_locking
Closed

bpo-43468: Per instance locking for functools.cached_property #27609
rhettinger wants to merge 16 commits intopython:mainfrom
rhettinger:cached_property_locking

Conversation

@rhettinger
Copy link
Copy Markdown
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
Copy Markdown
Member

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

# 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
Copy Markdown
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.

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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