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

[#217,#218] Remove Python 2 support code, implement proper thread state handling (main) #219

Merged
merged 2 commits into from
Aug 22, 2024

Conversation

SwooshyCueb
Copy link
Member

@SwooshyCueb SwooshyCueb commented Aug 16, 2024

Addresses #217
Addresses #218 (WIP)

Not tested yet, could probably be improved.

Since I am currently testing with 4-3-stable, progress may be followed in #220 instead of in this PR.

@trel
Copy link
Member

trel commented Aug 16, 2024

just noticing the std::lock_guard is pulled out of the try/catch, in case that's important.

  1. Throws any exceptions thrown by m.lock().

@SwooshyCueb
Copy link
Member Author

just noticing the std::lock_guard is pulled out of the try/catch, in case that's important.

  1. Throws any exceptions thrown by m.lock().

There's a reason for that. I'll probably end up moving it back in, but I need to tweak some exception handling

@SwooshyCueb SwooshyCueb changed the title [WIP] [#218] Proper python multithreading [WIP] [#217,#218] Remove Python 2 support code, implement proper thread state handling Aug 19, 2024
@SwooshyCueb SwooshyCueb changed the title [WIP] [#217,#218] Remove Python 2 support code, implement proper thread state handling [WIP] [#217,#218] Remove Python 2 support code, implement proper thread state handling (main) Aug 19, 2024
@SwooshyCueb SwooshyCueb marked this pull request as draft August 19, 2024 17:11
@SwooshyCueb
Copy link
Member Author

SwooshyCueb commented Aug 19, 2024

Since I am currently testing with 4-3-stable, progress may be followed in #220 instead of in this PR.

@trel
Copy link
Member

trel commented Aug 19, 2024

this is #219. i think you're suggesting #220

@SwooshyCueb SwooshyCueb force-pushed the 218.main branch 2 times, most recently from 8d17a51 to f61c636 Compare August 20, 2024 19:57
@SwooshyCueb SwooshyCueb changed the title [WIP] [#217,#218] Remove Python 2 support code, implement proper thread state handling (main) [#217,#218] Remove Python 2 support code, implement proper thread state handling (main) Aug 21, 2024
@SwooshyCueb SwooshyCueb marked this pull request as ready for review August 21, 2024 00:29
With Python 3.12, we started seeing crashes during certain multithreaded
operations. It turns out that changes to the GIL in 3.12 exposed that we are
not handling Python thread state properly. This commit fixes this.

A new namespace python_state has been added to main.cpp for holding onto some
Python state information, and a new struct python_thread_state_scope has also
been added for managing Python thread state.

This will be improved upon in the future.
@SwooshyCueb
Copy link
Member Author

#'d

@korydraughn
Copy link
Contributor

Is this identical to the PR for 4-3-stable?

@SwooshyCueb
Copy link
Member Author

Yes

@alanking alanking merged commit bcf1d13 into irods:main Aug 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants