gh-152754: Fix crash when an os.scandir iterator is shared between threads

[deadlovelll]

Fix crash when an os.scandir iterator is shared between threads

for more details see gh-152754

[nascheme]

I have some concerns about this approach. Using critical sections is probably not the right tool. We still want to release the GIL when doing IO, i.e. the Py_BEGIN_ALLOW_THREADS / Py_END_ALLOW_THREADS blocks. I created a revised PR based on this one that uses a mutex instead. With the mutex it's easier to confirm that everything is safe (critical section can be released depending on what you call inside of it) and it's pretty easy to also confirm there is no deadlock risk (always a concern when using a plain mutex).

BTW, for the unit test, you should use threading_helper.run_concurrently().

[deadlovelll]

I have some concerns about this approach. Using critical sections is probably not the right tool. We still want to release the GIL when doing IO, i.e. the Py_BEGIN_ALLOW_THREADS / Py_END_ALLOW_THREADS blocks. I created a revised PR based on this one that uses a mutex instead. With the mutex it's easier to confirm that everything is safe (critical section can be released depending on what you call inside of it) and it's pretty easy to also confirm there is no deadlock risk (always a concern when using a plain mutex).

BTW, for the unit test, you should use threading_helper.run_concurrently().

Hello! Thanks for the review and for your version of the fix. Mutex version looks good, I didn't take into account the GIL releasing in non-ft builds.

So, i'd be happy for your PR to be merged, i'll mark mine as draft. Thanks!