Skip to content

gh-114940: Add a Per-Interpreter Lock For the List of Thread States #127037

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

Closed

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Nov 19, 2024

The new lock may be used instead of the global "HEAD" lock. That global lock guards the list of interpreters, but unfortunately also gets used as a generic "lock everything". This change helps us move away from that situation.

FYI, this is very similar to gh-125561.

Comment on lines +1757 to +1761
HEAD_LOCK(runtime);
if (runtime->stoptheworld.requested) {
decrement_stoptheworld_countdown(&runtime->stoptheworld);
}
HEAD_UNLOCK(runtime);
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is correct, but need to double-check.

@rruuaanng
Copy link
Contributor

It seems that there is an error in CI, I will refer to your implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

If I remember correctly, this was an issue that could not be changed because it would cause unexpected issues with CI (it was originally changed in my PR)

@ericsnowcurrently
Copy link
Member Author

I have factored out gh-127077 and will later update this PR on top of it.

@ericsnowcurrently
Copy link
Member Author

I ended up splitting up these changes into #127077 and #127115, partly to make the change cleaner and to sort out the cases where we still have to use HEAD_LOCK().

@ericsnowcurrently ericsnowcurrently deleted the interp-threads-lock branch November 21, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants