| msg104129 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-04-24 20:47 |
| In 3.x, SSL sockets are created by dup()ing the original socket and then closing it. No attempt is made to conserve the socket's characteristics, such as the timeout and probably other flags. I understand that it may be too late to change the design decision of using dup() and closing the original, but perhaps we should make a best effort to retain the original socket's characteristics. Or perhaps this limitation should be clearly documented (since especially 2.x works differently). |
|
|
| msg104130 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-04-24 21:11 |
| It is actually a little funnier. dup() preserves the blocking/non-blocking nature of the underlying OS socket, but not the "timeout" of the Python socket. As such, a "blocking-with-timeout" Python socket gets replaced with a truely non-blocking socket. |
|
|
| msg104131 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-04-24 21:20 |
| Another consequence is that the following check in __init__: timeout = self.gettimeout() if timeout == 0.0: # non-blocking raise ValueError("do_handshake_on_connect should not be specified for non-blocking sockets") could never get triggered since the timeout is reset to None by virtue of creating a new socket object. |
|
|
| msg104133 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-04-24 22:10 |
| I committed a fix+tests for the timeout value in r80456 (py3k) and r80457 (3.1). Apparently the socket objects' own dup() method doesn't try to retain anything else than the timeout. I'm leaving this issue as "pending" in case criticism or better options are provided :) |
|
|
| msg104138 - (view) |
Author: Jean-Paul Calderone (exarkun) * |
Date: 2010-04-25 01:21 |
| Well, at the risk of stating the obvious, perhaps the dup() thing should be eliminated. The justification for it seems less than clear, and apparently it causes some problems. That might be a direction to consider in the long term, though, rather than as a (different) immediate fix for this issue. |
|
|
| msg104281 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-04-26 21:47 |
| > Well, at the risk of stating the obvious, perhaps the dup() thing > should be eliminated. The justification for it seems less than clear, > and apparently it causes some problems. I've just found another problem while investigating the cause of some sporadic Windows failures: http://www.python.org/dev/buildbot/builders/x86%20XP-4% 203.1/builds/718/steps/test/logs/stdio I've reproduced it on an XP VM and the explanation is that, sometimes, just after a dup() of a socket, calling getpeername() on the child socket fails (while getpeername() on the parent succeeds). It seems very timing-sensitive: if I insert enough code after the dup(), the call to getpeername() succeeds. I will fix the buildbot issue by using a different logic (simply, call getpeername() on the parent rather than the child), but this seems to confirms that dup() may not be a good idea. |
|
|
| msg104285 - (view) |
Author: Jean-Paul Calderone (exarkun) * |
Date: 2010-04-26 23:14 |
| getpeername() "sometimes" failing "soon" after a socket is created is a semi-well-known Windows socket... feature. For whatever that's worth. |
|
|
| msg108212 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-06-19 22:14 |
| I have tried refactoring the ssl code in order not to inherit from socket anymore, but it turned out suboptimal because chunks of code from the socket class then have to be duplicated. Therefore, I instead propose a much simpler approach which is to add a forget() method to socket objects; this method "closes" the socket object (sets the internal fd to -1) without closing the underlying fd at all. This allows to create another socket (actually SSLSocket) from the same fd without having to dup() it. Here is a patch; if the principle is accepted, I will add tests and docs. |
|
|
| msg108214 - (view) |
Author: Jean-Paul Calderone (exarkun) * |
Date: 2010-06-19 22:36 |
| It might be nice to see the version that avoids the dup() and has the duplicate code instead (interesting trade-off ;). Just for the sake of comparison against the forget() proposal. |
|
|
| msg108248 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-06-20 20:51 |
| > It might be nice to see the version that avoids the dup() and has the > duplicate code instead (interesting trade-off ;). Just for the sake of > comparison against the forget() proposal. Here it is. There's probably a bit of polishing left to do, in the (unlikely) case this is chosen as a solution. |
|
|
| msg112861 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-08-04 18:56 |
| If nobody chimes in, I will opt for the socket.forget() solution. |
|
|
| msg113350 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-08-08 23:26 |
| sockforget.patch committed in r83869 (py3k). Thank you for the comments. |
|
|
| msg115682 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-09-06 01:43 |
| In case someone lands here from the 3.2 what’s new document: The method has been renamed to detach. |
|
|