Issue2320

classification
Title: fileno() is -1 on Windows
Type: behaviour Severity: normal
Components: Core Versions: Jython 2.7
Milestone:
process
Status: open Resolution: accepted
Dependencies: Superseder:
Assigned To: jeff.allen Nosy List: amak, jeff.allen, rsc1975, zyasoft
Priority: normal Keywords: test failure causes

Created on 2015-04-10.15:32:48 by jeff.allen, last changed 2019-05-02.19:47:35 by jeff.allen.

Messages
msg9792 (view) Author: Jeff Allen (jeff.allen) Date: 2015-04-10.15:32:47
>>> f = file('x.tmp', 'wb')
>>> fd = f.fileno()
>>> type(fd)
<type 'org.python.core.io.FileIO'>
>>> os.fstat(fd)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 9] Bad file descriptor
>>> int(fd)
-1
>>> f.close()

In this code:
https://hg.python.org/jython/rev/e1a246f6a178#l5.89
we get at the integer file descriptor (index) via the private member fd. In my experience (with the debugger) fd is only ever -1, but there is an extra member that holds a Windows file handle.

This is the cause of a failure in test_tempfile, but not elsewhere as I believe we have comprehensively adapted to file descriptors that are objects.

If we support integer file descriptors, don't we need a table for them to index, as in C (and CPython)?
msg9803 (view) Author: Jim Baker (zyasoft) Date: 2015-04-11.13:24:05
Integer file descriptors are presumed to be coming from C. I believe this is the right logic here - either we can access the file or not with an int file descriptor as known to the underlying C. Otherwise throw EBADF as it is doing.
msg9818 (view) Author: Jim Baker (zyasoft) Date: 2015-04-13.21:53:03
Jeff, I think we will need to special case somewhere in test_tarfile about being on Windows for this usage. I'm going to mark this as invalid unless we can come up with a different approach, which I would certainly welcome.
msg9832 (view) Author: Jeff Allen (jeff.allen) Date: 2015-04-14.07:00:41
I'm -1 on the whole integer file descriptors thing anyway. I see it present in CPython, but it's all a bit too FORTRAN for me.

We tried doing without them, and I think that nearly worked, but not quite. There was discussion of this on python-dev and BDFL support for this divergence, but it seems to have aged off everywhere. The essence was that integer FDs are a low-level intrusion from the platform and some kind of file-like object should always be preferred. Butthey keep popping up.

The idea that a file-like objects is convertible to an int is a step forward, but I don't see how we can complete the idea unless the reverse is also possible: each method that takes an fd is able to convert it to a file-like object through global mapping/function.

No need for a lecture here on the drawbacks of global shared data.

Short-term a skip if nt is sensible. You'll find other skips/ifs in the tests around use of fileno(), conditioned on is_jython that could now be conditional on os._name. But if inte fds are desirable for compatibility with CPython, then they're desirable cross-platform. At first sight, the Windows file handle is a substitute ... but I'm a tad uncomfortable: Java makes this stuff private for a reason.
msg9845 (view) Author: Jeff Allen (jeff.allen) Date: 2015-04-14.19:04:53
-0 is more accurate.

The test that fails is testing that an instance of SpooledTemporaryFile is not backed by a real file until you call fileno(), in a sequence of tests that several other methods either do or don't provoke this change of implementation. This gives me an idea.

Suppose we implement a table in Jython, of file-like objects (deliberately vague term). File-like objects are not entered by default (stdin, stout, stderr are), but they are entered when any action, like fileno(), requires them to have an integer fd. After that, one can look up the (raw) file by fd, for example in fdopen(). The index we assign is from that table, not as found in java.io.FileDescriptor. The difficulties associated with the shared global state are borne only by those who need the low-level facilities of the os module.
msg9857 (view) Author: Jim Baker (zyasoft) Date: 2015-04-14.23:32:13
Jeff, this does sound like a very workable proposal. So let's reopen and see if we can do this in a future release.
msg9870 (view) Author: Jeff Allen (jeff.allen) Date: 2015-04-16.05:25:55
Ok, I'll try that on 2.7.1 timescales, and we'll accept the divergence between platforms for 2.7.0.
msg10437 (view) Author: Jeff Allen (jeff.allen) Date: 2015-11-07.12:23:03
Well this is interesting but a bit frustrating.

My original complaint is perhaps slightly off beam: fileno() is not actually -1, since fileno() returns a variety of objects, but never (I think) an int. Rather this change:
https://hg.python.org/jython/rev/e1a246f6a178
ensures that fileno() often returns an object convertible to an int:
https://hg.python.org/jython/rev/e1a246f6a178#l4.49
If that int is not -1 it is used in calls to jnr-posix, otherwise we intend to use the Java FileDescriptor, e.g. here in fstat():
https://hg.python.org/jython/rev/e1a246f6a178#l5.459

That fails in practice on Windows because we do not arrange that the union actually contain the Java FileDescriptor, so jnr-posix is passed null. You'd think some test of fstat would fail, but no, and this is worth investigating separately.

The fstat failure is easily fixed here:
https://hg.python.org/jython/file/159a0fc6d562/src/org/python/modules/posix/PosixModule.java#l225
by adding "case -1: break;". And it works:

>>> f = io.open('x.tmp', 'wb')
>>> fd = f.fileno()
>>> os.fstat(fd)
nt.stat_result(st_mode=33188, st_ino=0, st_dev=0L, st_nlink=1, st_uid=0, st_gid=0, st_size=0, t_atime=1446677504, st_mtime=1446893184, st_ctime=1446677504)

... except that when we try to close the file afterwards, this happens:

>>> f.close()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\Jeff\Documents\Eclipse\jython-trunk\dist\Lib\_io.py", line 208, in close
    self.raw.close()
IOError: The handle is invalid

At present, this causes errors (during clean up) in 5 regression tests. I find the cause to be that the call to fstat has closed the underlying Windows file handle. (Calling os.fstat(0) has a really bad effect.) I think this is a bug in jnr-posix, and that it is fixed here:
https://github.com/jnr/jnr-posix/commit/36fd37dcd0e1eb48e28840a17796dd47fb94b9c1

So I will not be adding my "case -1:" just yet. I'll see if we might use later versions of JARs so as to get that jnr-posix fix. If that update causes no other regressions, I'll add my "case -1:" fix separately. And maybe all in time for 2.7.1.

This does not even touch yet the idea I floated that we could keep a correspondence between integer file descriptors and file-like objects. I've spent enough time stepping through the innards of inside jnr-posix, to wonder if that does not give us what we need, without us piling stuff on top. This is not for 2.7.1, however.
msg10445 (view) Author: Jeff Allen (jeff.allen) Date: 2015-11-09.22:32:04
This is a great bug: I so love the PosixModule. fstat() no longer blows up, but the st_mode is zero, which upsets the fileinput module (by creating read-only output files that test_fileinput can't delete).

However, I have a cunning plan.
History
Date User Action Args
2019-05-02 19:47:35jeff.allensetnosy: - marthasimons
2019-05-02 19:46:57jeff.allensetmessages: - msg12484
2019-05-02 12:59:36marthasimonssetnosy: + marthasimons
messages: + msg12484
2018-03-06 07:45:56jeff.allensetmilestone: Jython 2.7.2 ->
2015-12-29 23:55:02zyasoftsetmilestone: Jython 2.7.1 -> Jython 2.7.2
2015-11-30 20:55:28rsc1975setnosy: + rsc1975
2015-11-09 22:32:04jeff.allensetmessages: + msg10445
2015-11-07 12:23:04jeff.allensetmessages: + msg10437
2015-11-02 08:38:16amaksetnosy: + amak
2015-04-16 05:25:56jeff.allensetpriority: normal
assignee: jeff.allen
messages: + msg9870
2015-04-14 23:32:13zyasoftsetstatus: pending -> open
resolution: invalid -> accepted
messages: + msg9857
milestone: Jython 2.7.1
2015-04-14 19:04:53jeff.allensetmessages: + msg9845
2015-04-14 07:00:42jeff.allensetmessages: + msg9832
2015-04-13 21:53:03zyasoftsetstatus: open -> pending
resolution: invalid
messages: + msg9818
2015-04-11 13:24:05zyasoftsetnosy: + zyasoft
messages: + msg9803
2015-04-10 15:32:48jeff.allencreate