The patch adds support for _socket.fromfd() and _socket.socket.dup() on
Windows.  It uses the Win32 DuplicateHandle() function.

The patch is to socketmodule.c an test_socket.py.

I've created a clean patch and tested it. It works as promised, great
work! Somebody with more Windows Fu than me should verify it before it's
applied.

I'd like to see this applied, HOWEVER I'm confused.  I don't see any
mention of fromfd in the patch except in a comment.  Now, in 3.0,
fromfd() is implemented in socket.py using os.dup(); but in 2.6 i don't
see it there either.  What's going in?

I am hoping that this will help issue 1439 along as well.

Never mind. I must've searched for fromdf.  Crys, I think this is good
to go into 2.6 and be merged into 3.0.  Then in 3.0 we can simplify Bill
Janssen's patch further.

Neal, Thomas, what do you think about the patch? Your knowledge of the
Windows API is greater than mine. Is the duplicate_socket() function ok?
I don't want to apply a patch I don't fully understand.

+#ifdef MS_WINDOWS
+/* On Windows a socket is really a handle not an fd */
+static SOCKET
+duplicate_socket(SOCKET handle)
+{
+       HANDLE newhandle;
+
+       if (!DuplicateHandle(GetCurrentProcess(), (HANDLE)handle,
+                            GetCurrentProcess(), &newhandle,
+                            0, FALSE, DUPLICATE_SAME_ACCESS))
+       {
+               WSASetLastError(WSAEBADF);
+               return INVALID_SOCKET;
+       }
+       return (SOCKET)newhandle;
+}
+#define dup(fd) duplicate_socket(fd)
+#define SOCKETCLOSE closesocket
+#define NO_MAKEFILE /* socket handles can't be treated like file handles */
+#endif

Port of the socket_fromfd.patch to py3k

Christian Heimes schrieb:
> Neal, Thomas, what do you think about the patch? Your knowledge of the
> Windows API is greater than mine. Is the duplicate_socket() function ok?
> I don't want to apply a patch I don't fully understand.
> 
> +#ifdef MS_WINDOWS
> +/* On Windows a socket is really a handle not an fd */
> +static SOCKET
> +duplicate_socket(SOCKET handle)
> +{
> +       HANDLE newhandle;
> +
> +       if (!DuplicateHandle(GetCurrentProcess(), (HANDLE)handle,
> +                            GetCurrentProcess(), &newhandle,
> +                            0, FALSE, DUPLICATE_SAME_ACCESS))
> +       {
> +               WSASetLastError(WSAEBADF);
> +               return INVALID_SOCKET;
> +       }
> +       return (SOCKET)newhandle;
> +}
> +#define dup(fd) duplicate_socket(fd)
> +#define SOCKETCLOSE closesocket
> +#define NO_MAKEFILE /* socket handles can't be treated like file handles */
> +#endif

Not much time, so only one comment:
Is it wise to call WSASetLastError(WSAEBADF) instead of calling GetLastError()?

Thomas

I'm disappointed that this doesn't implement socket.fromfd() properly.
Perhaps fromfd() needs to be implemented in socketmodule.c?

Also, the whole mess with the _base slot can be gotten rid of;
accept() can use the dup() method instead.

> I'm disappointed that this doesn't implement socket.fromfd() properly.
> Perhaps fromfd() needs to be implemented in socketmodule.c?

On Windows socket.fromfd() is not possible. Socket handlers and file
descriptors are different types using different API methods. Other
examples for the discrepancy are select.select() and select.poll(). On
Windows they don't work on file.

> On Windows socket.fromfd() is not possible. Socket handlers and file
> descriptors are different types using different API methods. Other
> examples for the discrepancy are select.select() and select.poll(). On
> Windows they don't work on file.

I know, but, I was thinking of it taking the fileno() of some other
socket object as argument.

In any case I think the test should not be based on whether os.name ==
'nt' but on the presence of something in the _socket module.

Crys, did you actually run the tests for the 3.0 port of the patch? I
think accept() is broken, as it checks for _can_dup_socket and will
attempt to call os.dup().

I'm thinking that we should change the C class to not create new objects
on accept() and dup() -- instead, there should be methods _accept() and
_dup() that return new file descriptors.  Then the subclass can
implement accept() and dup() properly without needing to call dup()
again, and the _base slot will no longer be needed.  Tell you what, I'll
try this and submit a patch for testing on Windows later.

Here's a new version along the lines I described. Please test on
Windows. It works on Linux and OSX. What this includes:

- the dup-on-windows implementation by R Oudkerk, with the
GetLastError() suggestion from Thomas Heller.

- Replace the C implementations of dup() and accept() with _dup() and
_accept() that return file descriptors instead of socket objects.

- Implement accept() and dup() in Python using _accept() and _dup().

- Get rid of socket.fromfd(). You can use socket.socket(..., fileno=...)
instead (it doesn't dup though).

New version of socket.diff.  The only change is better docstrings for
accept() and dup().

PS. socket.diff is for 3.0. I'll backport once this is accepted.

I've reviewed your patch and merged it was some pending changes of my
own. The socket tests are passing on Windows. Great work :)

UPDATE:
* Added PyLong_FromSocket_t and PyLong_AsSocket_t to longobject.h to get
rid of some 64bit warnings
* Use name dup_socket() instead of dup(). For Windows developers it's
too confusing to name it dup().
* Readded fromfd, but this time in C again.

From Guido's patch:
>        if (!DuplicateHandle(GetCurrentProcess(), (HANDLE)handle, 
>                             GetCurrentProcess(), &newhandle,
>                             0, FALSE, DUPLICATE_SAME_ACCESS)) 
>        {
>                WSASetLastError(GetLastError());
>                return INVALID_SOCKET;
>        }

If you are going to use GetLastError() like that then you really
should change set_error() so that it recognizes non-WSA errors.

The solution is easy: rip out the 80+ lines of Windows specific code
in set_error() and replace them by the much simpler

#ifdef MS_WINDOWS
        int err_no = WSAGetLastError();
        /* PyErr_SetExcFromWindowsErr() invokes FormatMessage() which
           recognizes the error numbers used by both GetLastError() 
           and WSAGetLastError() */
        if (err_no)
            return PyErr_SetExcFromWindowsErr(socket_error, err_no);
#endif

Note that if you make makefile() use a duplicate socket then you can
also remove all the reference counting stuff from the socket subclass.

roudkerk, can you turn that suggestion into a proper patch?

> I've reviewed your patch and merged it was some pending changes of my
> own. The socket tests are passing on Windows. Great work :)

You didn't upload this though.

Oh, I forgot

- I'm hoping that Bill can submit his SSL changes first.

- If we make _dup() a module-level function, we can implement fromfd()
in Python. I'll do this now.

socket2.diff makes dup() a module-level function in _socket (and hence
in socket.py) and uses that to implement fromfd() in socket.py.

No changes otherwise compared to py3k_another_socket.patch.

socket3.diff builds on socket2.diff:

- Rename socket class to Socket; add socket() as a factory function.

- Implement roudkerk's suggestion of using a duplicate socket in
makefile() to get rid of the manual reference counting. Yay!

Note: this dinterferes with Bill Janssen's issue 1451.  Whichever is
checked in last must fix issues with the other.

Forgot to upload socket3.diff.

I've yet another idea for a tiny improvement. Instead of doing

   newfd = _socket.dup(socket_instance.fileno())

I prefer

   newfd = _socket.dup(socket_instance)

It removes some confusing magic and makes error checking on Windows
slightly easier.

Hold on, socket3.diff breaks four unit tests:

test_ftplib test_poplib test_smtplib test_urllib2net

> newfd = _socket.dup(socket_instance)

But that doesn't allow fromfd to work. I found some real use cases for
fromfd where the fd is passed in through some other means:

http://www.google.com/codesearch?as_q=socket%5C.fromfd&btnG=Search+Code&as_lang=python

Currently on Windows set_error() make use of a large array which maps 
socket error numbers to error messages.

This patch (against socketmodule.c from Python 2.6) removes that 
array and just lets PyErr_SetExcFromWindowsErr() 
generate the message by using the Win32 function FormatMessage().

It is orthogonal from the other patches discussed here.

Thanks!  roudkerk's patch is submitted as revision 59004.

BTW I need to go back to the drawing board for the rest of the socket
patch here.  Using dup() in makefile() doesn't work for the
ssl.SSLSocket class.  Maybe the explicit reference counting is the best
we can get.

I've submitted socket2.diff plus small changes to ssl.py.
This seems the best I can do given that I don't think I can make dup()
work on ssl sockets.