When you're done making the requested changes, leave the comment:I have made the requested changes; please review again.

🤖 New build scheduled with the buildbot fleet by@kumaraditya303 for commitab1baf4 🤖

If you want to schedule another build, you need to add the🔨 test-with-buildbots label again.

I am not sure how it is not needed. I see similar thing done in_csv module.

Hm, Serhiy's general fix (gh-22870) was committed just before the _csv extension module was adapted (gh-23224).

When I try pickling with protocols 0 and 1 in the REPL, I get a correct TypeError for the _io types. It's strange that this is not the behaviour we're also seeing intest_pickling.

When I try pickling with protocols 0 and 1 in the REPL, I get a correct TypeError for the _io types. It's strange that this is not the behaviour we're also seeing in test_pickling.

Did you remove the_PyIOBase_cannot_pickle before doing that? I tried and it pickled without error. Anyways I am not really an expert on pickle.

Did you remove the_PyIOBase_cannot_pickle before doing that? I tried and it pickled without error. Anyways I am not really an expert on pickle.

Probably not; I tried again yesterday, and it also pickled without error. I'm not an expert on pickle either, but it seems to me we've found a corner case where Serhiy's fix of 2020 does not apply. One option can be to fixLib/copyreg.py. In any case, I suggest to land this PR with the pickleworkaround in it. If we end up patching Serhiy's general pickle fix, we can revert the workaround introduced here (as Serhiy's patch did with other extension modules back in 2020).

Agreed, I'll make a final review and merge by later today.

Finally!

It's good to see_PyIO_get_module_state() going away :-) Also, bye bye mystatic_types and_PyIO_FiniTypes() workarounds!

I'm happy that these changes were merged as a long list of changes (see PR list in issue#101819): if something will go wrong, it will be easier to identify which sub-part of these changes is causing troubles. I expect troubles, but that's fine. We already have to go trough turbulences to modernize Python ;-)

Thanks@erlend-aasland and@kumaraditya303 for your hard work on this complicated _io extension.

🤖 New build scheduled with the buildbot fleet by@sunmy2019 for commitfe2db1b 🤖

If you want to schedule another build, you need to add the🔨 test-with-refleak-buildbots label again.

@sunmy2019, why did you engage the refleak bots on a closed PR? Did you check the results of the refleak run that Kumar did for the same commit (fe2db1b) yesterday first? (All 31 checks passed.)

It should not pass the ref leaks tests.#104510 Just rerun to check what's going on.

Also, I should not trigger builtbot on a merged PR. It seems nothing will be tested. That's my bad.

Also, I should not trigger builtbot on a merged PR. It seems nothing will be tested. That's my bad.

Yes. And you can examine the refleak run Kumar did post merge. I did yesterday. I'm not sure why that passed, though.

Yes. And you can examine the refleak run Kumar did post merge. I did yesterday. I'm not sure why that passed, though.

Your PR was created in February. Maybe the code evolved in the meanwhile and the final "squash + rebase" commit is different than the PR branch ran on buildbots. For such PR which is in the works for a long time, I prefer to manually squash+rebase time to time to workaround this workflow limitation. There are services likehttps://mergify.com/ which prevent this workflow flaw. I heard that GitHub automerge can do something similar (run GHA jobs on the "final" commit), but buildbots are not handled and use the old way (run the jobs on the PR branch which is not merged into main / rebased).