classification
Title: os.fdlistdir() is not idempotent
Type: behavior Stage: resolved
Components: None Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: neologix, pitrou, python-dev, rosslagerwall
Priority: normal Keywords: patch

Created on 2012-01-08 16:26 by neologix, last changed 2012-01-09 19:42 by neologix. This issue is now closed.

Files
File name Uploaded Description Edit
fdlistdir.diff neologix, 2012-01-08 16:26 review
Messages (7)
msg150877 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2012-01-08 16:26
After a call to fdlistdir(), another call to fdlistdir() on the same file handle (but using a different FD, since the FD passed to fdlistdir() is closed) will return an empty list:

"""
$ cat ~/test_fdlistdir.py 
import os
import sys


fd = os.open(sys.argv[1], os.O_RDONLY)

print(os.fdlistdir(os.dup(fd)))
print(os.fdlistdir(os.dup(fd)))

os.close(fd)
$ ./python ~/test_fdlistdir.py /tmp/
['pulse-B1FebW397VI5', 'ksocket-kdm', 'etc', 'kde-cf', 'ksocket-cf', 'test_posix.py', '.X0-lock', 'kde-kdm', 'akonadi-cf.k6y52j', 'ssh-iSFleEAS1243', '.ICE-unix', '.X11-unix']
[]
"""

That's because fdopendir()/readdir doesn't reset the FD offset.
It's documented by POSIX:
"""
The file offset associated with the file descriptor at the time of the call determines which entries are returned.
"""

That's rather suprising (I got bitten while trying to write a test for #13734).
I see two options:
1. rewind the directory stream in fdlistdir()
2. document this

Here's a patch for option 1.
msg150879 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-08 17:18
> I see two options:
> 1. rewind the directory stream in fdlistdir()
> 2. document this
> 
> Here's a patch for option 1.

Agreed with that, and ok with the patch :)
msg150882 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2012-01-08 17:32
> I see two options:
> 1. rewind the directory stream in fdlistdir()
> 2. document this
> 
> Here's a patch for option 1.

Yeah, looks good.
msg150883 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-01-08 17:34
New changeset 7b2a178c028b by Charles-François Natali in branch 'default':
Issue #13739: In os.listdir(), rewind the directory stream (so that listdir()
http://hg.python.org/cpython/rev/7b2a178c028b
msg150887 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-01-08 18:07
New changeset 36f2e236c601 by Charles-François Natali in branch 'default':
Issue #13739: It's simpler and more direct to call rewinddir() at the
http://hg.python.org/cpython/rev/36f2e236c601
msg150898 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-08 19:13
For some reason, the second changeset broke the OpenIndiana buildbots:
http://www.python.org/dev/buildbot/all/builders/AMD64%20OpenIndiana%203.x/builds/2485

Also, wouldn't it be better to call rewinddir with the GIL released?
(although I agree rewinddir shouldn't be expensive)
msg150899 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2012-01-08 19:29
> For some reason, the second changeset broke the OpenIndiana buildbots:
>

I have absolutely no idea of why this doesn't work. I suspect
rewinddir() is a noop on OpenIndiana if readdir() hasn't been called.
I'll revert this commit.

> Also, wouldn't it be better to call rewinddir with the GIL released?
> (although I agree rewinddir shouldn't be expensive)

rewinddir() just changes a pointer, and calls - or ought to call -
lseek() on the FD. This should be fast, since no I/O is involved
(lseek() is not documented to return EINTR, for example). Releasing
the GIL has a cost :-)
History
Date User Action Args
2012-01-09 19:42:29neologixsetstatus: open -> closed
resolution: fixed
stage: resolved
2012-01-08 19:29:03neologixsetmessages: + msg150899
2012-01-08 19:13:47pitrousetmessages: + msg150898
2012-01-08 18:07:42python-devsetmessages: + msg150887
2012-01-08 17:34:43python-devsetnosy: + python-dev
messages: + msg150883
2012-01-08 17:32:07rosslagerwallsetmessages: + msg150882
2012-01-08 17:18:31pitrousetmessages: + msg150879
2012-01-08 16:26:25neologixcreate