Author martin.panter
Recipients Arfrever, BreamoreBoy, dfarrell07, martin.panter, ned.deily, orsenthil, python-dev, r.david.murray, vstinner
Date 2016-04-01.09:50:29
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1459504229.69.0.00970134744354.issue21069@psf.upfronthosting.co.za>
In-reply-to
Content
Mark: My understanding is on Windows, winsock file descriptors and C library file descriptors are different beasts; see <https://docs.python.org/3/library/socket.html#socket.socket.fileno>. Perhaps the test should call socket functions like socket.recv() on the FD rather than C library functions via os.fdopen().

Victor: The test in this bug has started failing again, very likely due to your revision 7bd4736195ce enabling a timeout on the HTTP request. I guess this causes the socket to be in non-blocking mode, and read() to return None. This is what Issue 10119 tried to fix. Example:

http://buildbot.python.org/all/builders/AMD64%20Debian%20root%203.x/builds/3404/steps/test/logs/stdio
======================================================================
FAIL: test_fileno (test.test_urllibnet.urlopenNetworkTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_urllibnet.py", line 110, in test_fileno
    self.assertTrue(f.read(), "reading from file created using fd "
AssertionError: None is not true : reading from file created using fd returned by fileno failed

A less serious and long-standing problem with the test is that it attempts to close the socket twice. We are just lucky that socket.close() is called second, which does not raise any errors: <https://hg.python.org/cpython/annotate/8938b3132caa/Modules/socketmodule.c#l259>.

Regarding the purpose and use cases of fileno(), I agree with Senthil that using it to read the HTTP response behind the HTTPResponse object’s back in Python 3 is a bad idea, and I don’t think it is practical to make this work without losing the benefits of buffering. But there are probably other valid use cases such as calling getsockname() on the socket, or sending and receiving non-HTTP data after setting up a CONNECT tunnel.

Proposals:

1. Change the test to do use socket(fileno=...), rather than os.fdopen(...), so that it will be usable on Windows.

2. Ensure that the secondary socket object is not closed; use socket.detach()

3. Rewrite the test to test http.client directly, rather than indirectly through urlopen(). As far as I can see the purpose is only to test HTTPResponse.fileno(), not urlopen().

4. Rewrite the test to test a local server run in a background thread, rather than relying external web sites (currently Google, previously IETF, and Python). This would eliminate the need for setting a timeout.

5. Rewrite to the test for a more realistic use case that does not depend on specific internal HTTPResponse buffering and the HTTP protocol. I suggest mocking a CONNECT request, and uploading some non-HTTP data through the proxy.
History
Date User Action Args
2016-04-01 09:50:29martin.pantersetrecipients: + martin.panter, orsenthil, vstinner, ned.deily, Arfrever, r.david.murray, BreamoreBoy, python-dev, dfarrell07
2016-04-01 09:50:29martin.pantersetmessageid: <1459504229.69.0.00970134744354.issue21069@psf.upfronthosting.co.za>
2016-04-01 09:50:29martin.panterlinkissue21069 messages
2016-04-01 09:50:29martin.pantercreate