Title: gh-157841: Synchronize access to functools.partial's mutable state by Adarsh-Me · Pull Request #157979 · python/cpython · GitHub
Open Graph Title: gh-157841: Synchronize access to functools.partial's mutable state by Adarsh-Me · Pull Request #157979 · python/cpython
X Title: gh-157841: Synchronize access to functools.partial's mutable state by Adarsh-Me · Pull Request #157979 · python/cpython
Description: Bug report Issue: 157841 Type: crash / data race in a free-threaded build functools.partial mutates its own state from __setstate__() with no synchronisation against the readers of that state. Under --disable-gil the fields (func, args, keywords, phcount) and the vectorcall slot are published as independent stores, so a reader can combine two different states, and the displaced values' decrefs can run arbitrary code while the object is half-updated. Coordinating with @espressolee on #157841, who pointed out that the readers (partial_repr, partial_call, partial_vectorcall) take no critical section either at the current head — so making only the writer atomic would leave a reader sampling each field independently. Two separate invariants had to hold: lifetime — Py_NewRef(pto->args) loads then increments, so a concurrent replacement can drop the last reference in between; the acquisition itself has to be inside the critical section, not just the use; coherence — partial_vectorcall samples phcount and args separately and combines them into tot_nargs, so both have to come from one state. How to fix the bug Every access path takes a critical section on the partial and reads or writes the whole state from it: partial_setstate() builds the replacement values first, then installs func, args, keywords, phcount and the vectorcall pointer as one unit, and only drops the displaced references after the new state is fully published (their __del__/finalisers can re-enter and read the partial); partial_repr(), partial_call() and partial_vectorcall() take the same section, copy out the strong references and phcount, then do the formatting or the call after leaving it; partial_new()'s flatten path reaches inside its argument to copy func/args/keywords, which is a reader like any other, so it goes through the same protocol and owns the references it takes. PR feature rule This is a bugfix, not a new feature. Core Dev guide Devguide: add tests for bugs (not applied to cosmetic changes) Docs & Tools Documentation changes: not needed, this is a concurrency fix in an existing method's behaviour. Verification Free-threaded amd64t Release build of this branch (MSC v.1929, 3.16.0a0), Windows. The two new TestPartial cases were run against two builds that differ only in Modules/_functoolsmodule.c: build python -m test test_free_threading.test_functools -v main's _functoolsmodule.c, new tests present test_concurrent_setstate_and_reads → Fatal Python error: PyMutex_Unlock: unlocking mutex that is not locked this branch Ran 7 tests ... OK, Result: SUCCESS The tests are 10 threads × 1000 operations with sys.setswitchinterval(1e-6), covering repr + __reduce__ + call + partial(p, 3) flattening against a concurrent __setstate__, and they check a torn read rather than only a crash: every state written carries the same value twice, once positionally and once as a keyword, so a mixture of two states fails an assertion instead of happening to survive. What I did not measure: a writer-only patch (critical section in partial_setstate() but not in the readers), which is the shape @espressolee rightly says would not give readers a coherent snapshot. Happy to add that data point if it is useful, and to run anything on a Linux build — the numbers above are Windows-only. Issue: gh-157841
Open Graph Description: Bug report Issue: 157841 Type: crash / data race in a free-threaded build functools.partial mutates its own state from __setstate__() with no synchronisation against the readers of that state. Un...
X Description: Bug report Issue: 157841 Type: crash / data race in a free-threaded build functools.partial mutates its own state from __setstate__() with no synchronisation against the readers of that state. Un...
Opengraph URL: https://github.com/python/cpython/pull/157979
X: @github
Domain: github.com
| react-profiling | 0 |
| react-import-map | react,react-dom,react-dom/client,react-dom/profiling,react-is,react-reconciler,react/compiler-runtime,react/jsx-dev-runtime,react/jsx-runtime,scheduler@d5750093facf |
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:d7cb8043-70ef-a904-5351-15e4ab869098 |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | A4C0:2314:B84366:F47899:6AF85E6F |
| html-safe-nonce | ac864773dd508d9c62094c84e1a2ca0d1bf8d7de6c17046cf2cd2acf0842ce3b |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJBNEMwOjIzMTQ6Qjg0MzY2OkY0Nzg5OTo2QUY4NUU2RiIsInZpc2l0b3JfaWQiOiIyNTIwNTk0NzM4NzY3NzQ4NzE5IiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | 698dfac3b0a9897c8ed7cf4e73ca50f88aacab48c2ff19f6c5dd22f51d7572fd |
| hovercard-subject-tag | pull_request:4611085657 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/python/cpython/pull/157979/files |
| twitter:image | https://avatars.githubusercontent.com/u/122873385?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/122873385?s=400&v=4 |
| og:image:alt | Bug report Issue: 157841 Type: crash / data race in a free-threaded build functools.partial mutates its own state from __setstate__() with no synchronisation against the readers of that state. Un... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 5713ecd83001b2f55c3509fb7a292bf7d66d0f167c7ae87a9f7eaec5a52bf8db |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/python/cpython git https://github.com/python/cpython.git |
| octolytics-dimension-user_id | 1525981 |
| octolytics-dimension-user_login | python |
| octolytics-dimension-repository_id | 81598961 |
| octolytics-dimension-repository_nwo | python/cpython |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 81598961 |
| octolytics-dimension-repository_network_root_nwo | python/cpython |
| turbo-body-classes | logged-out env-production page-responsive full-width |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 653be32ff5c755082a4db662491bfbbcfc9112c1 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width