classification
Title: os.listdir randomly fails on occasions when it shouldn't
Type: behavior Stage:
Components: Extension Modules Versions: Python 2.5
process
Status: closed Resolution: duplicate
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, draghuram, georg.brandl, philipspencer
Priority: normal Keywords: patch

Created on 2008-06-14 22:42 by philipspencer, last changed 2008-07-16 21:32 by georg.brandl. This issue is now closed.

Files
File name Uploaded Description Edit
python-2.5.1-listdir.patch philipspencer, 2008-06-15 22:35 The patch
Messages (6)
msg68223 - (view) Author: (philipspencer) Date: 2008-06-14 22:42
Python's os.listdir function has the following code in
Modules/posixmodule.c:

   errno = 0
   ...
    for (;;) {
         Py_BEGIN_ALLOW_THREADS
         ep = readdir(dirp);
         Py_END_ALLOW_THREADS
         if (ep == NULL)
              break;
         ...
         a bunch of other stuff, including PyString_FromStringAndSize
         which calls malloc
         ...
    }
    if (errno != 0 && d != NULL) {

The assumption is that errno will be nonzero only if readdir failed.
However, this is not the case. GLibc's malloc will, in some rare cases,
set errno even when it succeeds. So, during one pass through the loop
errno gets set to ENOMEM and then it is still set to ENOMEM when the
final readdir returns null at the end of the directory listing.

The fix is to move the line "errno = 0" from outside the loop to
right before the readdir inside the loop. That is the only way to
ensure that, when the loop is exited with readdir returning null,
the condition "errno != 0" is equivalent to "readdir actually failed."

The attached patch does this. Without this patch, we experience
frequent failures with the python tools "creatrepo" and "repomanage"
when run on very large directories (> 5000 packages) on Fedora 8
systems; see RedHat bugzilla entry #451494. With the patch, the commands
work as expected.

The patch is against 2.5.1 but from what I can see of the posixmodule.c
code from SVN it should apply cleanly there too.
msg68246 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-06-15 19:48
You forgot the patch :-)
msg68252 - (view) Author: (philipspencer) Date: 2008-06-15 22:35
I submitted the patch using the File: pathname box in the issue tracker
editing window.

I am doing this from home using an older version of mozilla; perhaps the
tracker software doesn't work well with that version?

I will try one more time. If it still doesn't come through, the patch
can be viewed at

https://bugzilla.redhat.com/attachment.cgi?id=309374
msg68268 - (view) Author: Raghuram Devarakonda (draghuram) (Python triager) Date: 2008-06-16 13:57
Isn't this similar to #1608818?
msg68269 - (view) Author: (philipspencer) Date: 2008-06-16 14:37
Yes, it is the same issue. Sorry I didn't see the previous report -- I
didn't imagine an issue like this, with such a simple fix, could have
been reported back in 2006 without the fix ever having been implemented,
so I didn't bother searching back that far!

The fix in that report contains the same one-line fix as I proposed,
plus additional changes to ensure that the code won't break if in
the future additional changes are made to allow a second possible
exit-out-of-the-loop-with-d-being-non-null code path.

All that's strictly necessary to solve the current problem is the
one line I proposed, but the other proposed changes would make the
safer against future potential breakage if the code gets rearranged
later.
msg69841 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-07-16 21:32
Committed patch in #1608818 in r65037.
History
Date User Action Args
2008-07-16 21:32:13georg.brandlsetstatus: open -> closed
resolution: duplicate
messages: + msg69841
nosy: + georg.brandl
2008-06-16 14:37:29philipspencersetmessages: + msg68269
2008-06-16 13:57:22draghuramsetnosy: + draghuram
messages: + msg68268
2008-06-15 22:35:55philipspencersetfiles: + python-2.5.1-listdir.patch
keywords: + patch
messages: + msg68252
2008-06-15 19:48:44amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg68246
2008-06-14 22:42:57philipspencercreate