SSL sockets do not retain the parent socket's attributes

[pitrou]

BPO	8524
Nosy	@loewis, @pitrou, @giampaolo, @merwok
Files	sockforget.patch sslrefactor.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2010-08-08.23:26:12.440>
created_at = <Date 2010-04-24.20:47:46.535>
labels = ['library']
title = "SSL sockets do not retain the parent socket's attributes"
updated_at = <Date 2010-09-06.01:43:20.453>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2010-09-06.01:43:20.453>
actor = 'eric.araujo'
assignee = 'none'
closed = True
closed_date = <Date 2010-08-08.23:26:12.440>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2010-04-24.20:47:46.535>
creator = 'pitrou'
dependencies = []
files = ['17723', '17726']
hgrepos = []
issue_num = 8524
keywords = ['patch']
message_count = 13.0
messages = ['104129', '104130', '104131', '104133', '104138', '104281', '104285', '108212', '108214', '108248', '112861', '113350', '115682']
nosy_count = 6.0
nosy_names = ['loewis', 'exarkun', 'janssen', 'pitrou', 'giampaolo.rodola', 'eric.araujo']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue8524'
versions = ['Python 3.1', 'Python 3.2']

[pitrou]

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).

[pitrou]

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.

[pitrou]

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.

[pitrou]

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 :)

[exarkun]

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.

[pitrou]

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.

[exarkun]

getpeername() "sometimes" failing "soon" after a socket is created is a semi-well-known Windows socket... feature. For whatever that's worth.

[pitrou]

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.

[exarkun]

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.

[pitrou]

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.

[pitrou]

If nobody chimes in, I will opt for the socket.forget() solution.

[pitrou]

sockforget.patch committed in r83869 (py3k). Thank you for the comments.