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

Free-threading support #271

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

Conversation

FFY00
Copy link
Contributor

@FFY00 FFY00 commented Sep 20, 2024

WIP, don't merge.

Signed-off-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00
Copy link
Contributor Author

FFY00 commented Sep 20, 2024

The kwlist const change is not strictly required given how it is currently being used, but it's a good idea to avoid static variables when possible. This way the compiler will error out on bad changes, instead of potentially introducing bugs on free-threading builds.

The rest of the static variables are only used to construct str objects that shouldn't change, so that shouldn't introduce issues on free-threading builds. I don't think it's worth dropping the static modifier, as that would introduce performance hits, though very minor.

Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00
Copy link
Contributor Author

FFY00 commented Sep 20, 2024

From my review of the code, I didn't catch anything else that would be problematic on free-threaded builds — there's no global state other than the static variables mentioned above, and I didn't catch any other instances of unsafe borrowed reference usage.

I'll add some multi-threaded tests now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant