8000 Add locks around accesses/modifications to global encodings table by luke-gruber · Pull Request #13600 · ruby/ruby · GitHub
[go: up one dir, main page]
Skip to content

Add locks around accesses/modifications to global encodings table #13600

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

8000 Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add locks around accesses/modifications to global encodings table #13600

wants to merge 1 commit into from
+60 −18

Conversation

luke-gruber
Copy link
Contributor

This fixes segfaults and errors of the type "Encoding not found" when using encoding-related methods and internal encoding c functions across ractors.

@luke-gruber
Add locks around accesses/modifications to global encodings table
7792937 
This fixes segfaults and errors of the type "Encoding not found" when
using encoding-related methods and internal encoding c functions across
ractors.
@zzak
Copy link
Member
zzak commented Jun 16, 2025

Could you add a test and/or reproduction case?

Was there a ticket associated with this PR?

@jhawthorn
Copy link
Member

Could you add a test and/or reproduction case?

It tends to be difficult to make reliable reproductions to concurrency issues, and at the moment all Ractor tests need an assert_separately, which is less than desirable. It's better to make things safe based on correct design and in this case it's pretty obvious that we'll need to hold a lock to read or write to a global st_table (and the ASSERT_GLOBAL_ENC_TABLE_LOCKED added ensure this won't regress)

Was there a ticket associated with this PR?

As Ractors are experimental and there are many issues we aren't necessarily filing tickets as the fixes will not be backported.

@zzak
Copy link
Member
zzak commented Jun 16, 2025

Cool just checking, I saw this and thought neat but there was no bug associated or other context, so wasn't sure if it was ready for review or not. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@luke-gruber @zzak @jhawthorn
0