GIL is not released when dir_entry.stat does a stat syscall

[skonieczny]

We have an application that crawls filesystem using scandir. It uses multiple threads for various things. Application is used on variety of filesystems, some of them might be slow or ocasionally unresponsive.

We have found out that sometimes whole crawling process is stuck and no thread makes any progress, even threads that are really simple, makes no IO and do not hold any locks. After running py-spy on process that was stuck we saw that one of the threads has entered dentry.stat(follow_symlinks=False) line and still holds the GIL. Other threads are stuck, because they are waiting for the GIL. This situation can take a long time.

I think that DirEntry should release GIL when stat cache is empty and syscall is performed.

[benhoyt]

@skonieczny Just to confirm: are you running this on Python 2.7 using this scandir module, or Python 3.5+ using the built-in os.scandir function?

I don't think Python 3.5's DirEntry.stat releases the GIL either (see source). So there may be something else going on here ... or maybe Python's version should be releasing the GIL too.

Yeah, I think this might be a bug. Python's regular os.stat implementation does release the GIL: source. Maybe we can dig in a bit and report it against CPython too.

What operating system are you running? Are you able to reproduce this repeatably? If you're using this scandir library rather than the built-in one, are you able to add a couple of ALLOW_THREADS lines to _scandir.c and build and use that version? (I can tell you what to add.)

[skonieczny]

We are using python 3.7. Bug is both in python core os.scandir and in scandir.scandir. We initially hit the bug in in os.scandir. Then we tried to use scandir.scandir and we found the bug is also here. Both versions do not release a GIL.

I was able to create PR with a fix. It works for our case. It is here: https://github.com/benhoyt/scandir/pull/132 (#132).

I would like to fill a bug against cPython also, but I started here, as I suspected, I could got meaningful response earlier. I suppose it can be much faster to fix the bug here than to fix it in python core. However we are a little concerned if scandir.scandir is up to date with all fixes that might have been done in os.scandir. Do you have opinion on that?

[benhoyt]

Thanks! #132 looks good so far, though I added a comment there.

For the CPython version, it looks like there's only one code path for both Windows and POSIX -- it'd be great if you can submit a patch for CPython too. I think it's just wrapping these lines with the same begin/end-threads dance.

To answer your final question: yes, there have been a couple of improvements / fixes to the CPython os.scandir version that I haven't had time to pull in (and realistically probably won't get time). Notably support for the with statement and iterator .close(), and support for scandir() with a file descriptor as well as just a name. There may be other fixes too, I'm not sure. But if you're not using either of those features and this scandir module is working for you, that's okay too!

[benhoyt]

Cool, thanks for the update on #132. I'll close this scandir issue, but do post the link here when you've opened the bugs.python.org issue.

[skonieczny]

Python bug is here: https://bugs.python.org/issue45012

[benhoyt]

Nice work on the CPython fix, thanks!