Skip to content

Fixes to encoding/transcoding for ractors. #14295

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

Merged
merged 1 commit into from
Aug 22, 2025

Conversation

jhawthorn
Copy link
Member

Not all ractor-related encoding issues were fixed by 1afc07e. I found more by running my test-all branch with 3 ractors for each test.

This comment has been minimized.

@jhawthorn jhawthorn force-pushed the encoding_autoload_fixes branch from 4f20ac5 to 50b8e7a Compare August 21, 2025 19:07
@luke-gruber luke-gruber force-pushed the encoding_autoload_fixes branch 2 times, most recently from 82cf3af to 4f0f3a8 Compare August 21, 2025 22:46
@luke-gruber
Copy link
Contributor

luke-gruber commented Aug 21, 2025

The CI is fixed. The issue was actually with enc_arg in transcode.c:

Encoding::Converter.asciicompat_encoding
    econv_s_asciicompat_encoding
        enc_arg
            rb_to_encoding_index
                rb_enc_find_index
                    load_encoding // creates thread when multi ractor     

So, that test was creating 7x500=3500 threads, and was waiting for them sequentially. I lowered it to 7x50=350 threads, but even that might be high.

I tried removing the call to load_encoding in rb_enc_find_index, because I don't like it that we try to load encodings that aren't registered. It's been there since 2008 (0052259), and when I removed it test-all still passed. It's possible that all the other test suites also pass without it. We should at least have a test case that covers this behavior.

We could look into removing it, but I'm not sure that's possible as it would be a breaking change.

Not all ractor-related encoding issues were fixed by 1afc07e.
I found more by running my test-all branch with 3 ractors for each test.
@jhawthorn jhawthorn force-pushed the encoding_autoload_fixes branch from 4f0f3a8 to f378052 Compare August 22, 2025 17:16
@jhawthorn jhawthorn enabled auto-merge (rebase) August 22, 2025 17:16
@jhawthorn jhawthorn merged commit 9db54a1 into ruby:master Aug 22, 2025
84 checks passed
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.

2 participants