-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
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
gh-103583: Isolate CJK codec modules #103869
Conversation
erlend-aasland
commented
Apr 26, 2023
•
edited by bedevere-bot
Loading
edited by bedevere-bot
- Issue: Isolate _multibytecodec #103583
Lifetime should be guaranteed, since each MultibyteCodecObjects owns a reference the cjk codec module that owns the memory of the MultibyteCodec struct.
664f961
to
f6ed5df
Compare
This approach exploits the dependency introduced in gh-103589; we know for sure that the _codec* extension modules will outlive _multibytecodec. This means we can store the cjk codec module state safely in the codec struct and modify the various codec handlers to accept the codec struct (iso. |
🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit ef983c4 🤖 If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
BTW, I removed the |
@vstinner, you want to take a look? :) |
@erlend-aasland I will take a look by tomorrow |
(Pulling in main to get the latest ref. leak fixes by Jelle) |
🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit dec79a7 🤖 If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, This is quite a complicated but nice approach. But nothing can be better than this.
Thanks for the hard work!
Thanks for the review, Dong-hee! Yes, it is complicated, but as shown in my competing PRs, other approaches are even more complicated. |
Well done @erlend-aasland! |
Thanks, Victor :) This was a tricky one! |