Skip to content

Conversation

@ngoldbaum
Copy link
Member

Ref #26159

See also the CPython HOWTO on this topic: https://docs.python.org/3.13/howto/free-threading-extensions.html#freethreading-extensions-howto.

The remaining usages of PyDict_GetItem and PyDict_Next are all around the fields attribute of structured dtypes. I'm pretty sure that dictionary is effectively frozen after the DType is constructed, so I don't worry about those uses.

It's not straightforward to write tests for this, I'm just applying static refactorings in places where the refactoring shouldn't introduce new reference counting bugs.

@ngoldbaum ngoldbaum force-pushed the fix-c-api-thread-safety branch from 5414389 to c7e4b01 Compare August 8, 2024 21:11
@ngoldbaum ngoldbaum added 09 - Backport-Candidate PRs tagged should be backported 39 - free-threading PRs and issues related to support for free-threading CPython (a.k.a. no-GIL, PEP 703) labels Aug 8, 2024
Copy link
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

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

Second one seems unnnecessary to me, first one looks nice with PyDict_GetItemRef either way, I like it.
The last one seems good (I almost wonder if even a copy could make sense, but probably no reason to think about it).

Copy link
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

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

Thanks, now that I glance over, I half wonder if a goto solution can't be done nicely. (I don't love the error = state)

But, I am not sure, and it looks good, so if you don't think of something quickly, please just merge.

@ngoldbaum
Copy link
Member Author

The macro expands to statements with open and close braces so I couldn't think of a way to keep it syntactically sound and leave the gotos.

@seberg
Copy link
Member

seberg commented Aug 9, 2024

Ah, right, I hadn't realized that, thanks!

@seberg seberg merged commit 0273e05 into numpy:main Aug 9, 2024
charris pushed a commit to charris/numpy that referenced this pull request Aug 9, 2024
Ref numpy#26159

See also the CPython HOWTO on this topic: https://docs.python.org/3.13/howto/free-threading-extensions.html#freethreading-extensions-howto.

The remaining usages of PyDict_GetItem and PyDict_Next are all around the fields attribute of structured dtypes. I'm pretty sure that dictionary is effectively frozen after the DType is constructed, so I don't worry about those uses.

It's not straightforward to write tests for this, I'm just applying static refactorings in places where the refactoring shouldn't introduce new reference counting bugs.

* ENH: fix thread-unsafe C API usages

* ENH: use critical sections in einsum

* BUG: fix error handling in loadtxt C code

* revert einsum changes
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Aug 9, 2024
ArvidJB pushed a commit to ArvidJB/numpy that referenced this pull request Nov 1, 2024
Ref numpy#26159

See also the CPython HOWTO on this topic: https://docs.python.org/3.13/howto/free-threading-extensions.html#freethreading-extensions-howto.

The remaining usages of PyDict_GetItem and PyDict_Next are all around the fields attribute of structured dtypes. I'm pretty sure that dictionary is effectively frozen after the DType is constructed, so I don't worry about those uses.

It's not straightforward to write tests for this, I'm just applying static refactorings in places where the refactoring shouldn't introduce new reference counting bugs.

* ENH: fix thread-unsafe C API usages

* ENH: use critical sections in einsum

* BUG: fix error handling in loadtxt C code

* revert einsum changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

01 - Enhancement 39 - free-threading PRs and issues related to support for free-threading CPython (a.k.a. no-GIL, PEP 703)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants