classification
Title: Avoid using DuplicateHandle() on sockets in multiprocessing.connection
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: kristjan.jonsson, loewis, pitrou, python-dev, sbt
Priority: normal Keywords: patch

Created on 2012-04-07 19:02 by sbt, last changed 2012-04-09 11:56 by sbt. This issue is now closed.

Files
File name Uploaded Description Edit
mp_socket_dup.patch sbt, 2012-04-07 19:20 review
Messages (12)
msg157747 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-07 19:02
In multiprocessing.connection on Windows, socket handles are indirectly duplicated using DuplicateHandle() instead the WSADuplicateSocket().  According to Microsoft's documentation this is not supported.

This is easily avoided by using socket.detach() instead of duplicating the handle.
msg157750 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-04-07 19:09
What is the bug that this fixes? Can you provide a test case?
msg157751 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-07 19:20
> What is the bug that this fixes? Can you provide a test case?

The bug is using an API in a way that the documentation says is wrong/unreliable.  There does not seem to be a classification for that.

I have never seen a problem caused by using DuplicateHandle() so I cannot provide a test case.  Note that socket.dup() used to be implemented using DuplicateHandle(), but that was changed to WSADuplicateSocket().  See Issue 9753.
msg157752 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-07 19:27
Actually Issue 9753 was causing failures in test_socket.BasicTCPTest and test_socket.BasicTCPTest2 on at least one Windows XP machine.
msg157754 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-07 20:02
Is there a reason the patch changes close() to win32.CloseHandle()?
msg157757 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-07 20:12
> Is there a reason the patch changes close() to win32.CloseHandle()?

This is a Windows only code path so close() is just an alias for win32.CloseHandle().  It allow removal of the lines 

    # Late import because of circular import
    from multiprocessing.forking import duplicate, close
msg157759 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-07 20:44
New changeset f8a92fd084c2 by Antoine Pitrou in branch 'default':
Issue #14522: Avoid duplicating socket handles in multiprocessing.connection.
http://hg.python.org/cpython/rev/f8a92fd084c2
msg157760 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-07 20:45
Patch committed, thank you!
msg157797 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-04-08 17:06
multiprocessing.reduction still appears to use DuplicateHandle to copy sockets.
I propose adding a pair of custom functions to _multiprocessing, that "pickles" and "unpickles" handles.  It can detect socket handles as being different from e.g. pipe handles by using WSADuplicateSocket and return a bytes object, similar to what is already done in socketmodule (see issue 14310)
On non-windows, this would be a no-op.
_multiprocessing already linkes with winsock, whereas the subprocess is part of python core which doesn't.
msg157813 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-08 22:44
> multiprocessing.reduction still appears to use DuplicateHandle to copy sockets.

It's probably a separate issue, then. This one is about multiprocessing.connection :)
msg157836 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-04-09 10:38
possibly, multiprocessing.Connection uses handles, which can be socket handles on windows, and that code also uses DuplicateHandle.  I think a generic solution must be found for multiprocessing, so I'll create a separate issue.
msg157839 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-09 11:56
> I think a generic solution must be found for multiprocessing, so I'll 
> create a separate issue.

I have submitted a patch for Issue 4892 which makes connection and socket objects picklable.  It uses socket.share() and socket.fromshare() on Windows.
History
Date User Action Args
2012-04-09 11:56:22sbtsetmessages: + msg157839
2012-04-09 10:38:23kristjan.jonssonsetmessages: + msg157836
2012-04-08 22:44:36pitrousetstatus: open -> closed

messages: + msg157813
2012-04-08 17:06:02kristjan.jonssonsetstatus: closed -> open
nosy: + kristjan.jonsson
messages: + msg157797

2012-04-07 20:45:00pitrousetstatus: open -> closed
versions: - Python 3.2
messages: + msg157760

resolution: fixed
stage: patch review -> resolved
2012-04-07 20:44:38python-devsetnosy: + python-dev
messages: + msg157759
2012-04-07 20:12:19sbtsetmessages: + msg157757
2012-04-07 20:02:20pitrousettype: behavior
components: + Library (Lib)
versions: + Python 3.2, Python 3.3
nosy: + pitrou

messages: + msg157754
stage: patch review
2012-04-07 19:27:04sbtsetmessages: + msg157752
2012-04-07 19:20:31sbtsetfiles: + mp_socket_dup.patch

messages: + msg157751
2012-04-07 19:12:51sbtsetfiles: - mp_socket_dup.patch
2012-04-07 19:09:27loewissetnosy: + loewis
messages: + msg157750
2012-04-07 19:02:42sbtcreate