| msg57084 - (view) |
Author: roudkerk (roudkerk) |
Date: 2007-11-03 15:24 |
| 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. |
|
|
| msg57336 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2007-11-09 21:40 |
| 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. |
|
|
| msg57492 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-14 18:13 |
| 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. |
|
|
| msg57494 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-14 18:25 |
| 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. |
|
|
| msg57498 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-11-14 18:45 |
| 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 |
|
|
| msg57501 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-11-14 19:19 |
| Port of the socket_fromfd.patch to py3k |
|
|
| msg57504 - (view) |
Author: Thomas Heller (theller) * |
Date: 2007-11-14 19:28 |
| 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 |
|
|
| msg57505 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-14 19:33 |
| 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. |
|
|
| msg57506 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-11-14 20:30 |
| > 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. |
|
|
| msg57508 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-14 21:28 |
| > 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. |
|
|
| msg57511 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-14 23:13 |
| 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. |
|
|
| msg57513 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-15 00:00 |
| 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). |
|
|
| msg57514 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-15 00:46 |
| New version of socket.diff. The only change is better docstrings for accept() and dup(). |
|
|
| msg57515 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-15 00:47 |
| PS. socket.diff is for 3.0. I'll backport once this is accepted. |
|
|
| msg57516 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-11-15 01:55 |
| 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. |
|
|
| msg57541 - (view) |
Author: roudkerk (roudkerk) |
Date: 2007-11-15 18:14 |
| 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. |
|
|
| msg57548 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-15 18:44 |
| roudkerk, can you turn that suggestion into a proper patch? |
|
|
| msg57551 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-15 19:22 |
| > 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. |
|
|
| msg57552 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-11-15 19:33 |
| Oh, I forgot |
|
|
| msg57554 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-15 19:58 |
| - 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. |
|
|
| msg57555 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-15 20:21 |
| 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. |
|
|
| msg57556 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-15 20:36 |
| 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. |
|
|
| msg57557 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-15 20:36 |
| Forgot to upload socket3.diff. |
|
|
| msg57566 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-11-15 22:19 |
| 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. |
|
|
| msg57567 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-15 22:22 |
| 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 |
|
|
| msg57576 - (view) |
Author: roudkerk (roudkerk) |
Date: 2007-11-16 00:19 |
| 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. |
|
|
| msg57577 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-16 00:26 |
| 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. |
|
|
| msg57580 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-16 01:25 |
| 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. |
|
|
| msg93551 - (view) |
Author: Preston Landers (planders) * |
Date: 2009-10-04 17:39 |
| I'm curious what happened with this issue. It says closed+accepted but it doesn't appear to be checked in. Was there a fatal problem implementing this feature on Windows? Is it hung up on the inability to dup SSL sockets? I'm highly interested in deploying Python web servers using FastCGI, SCGI, WSGI, etc on the Windows server platform. I wrote a simple Windows fromfd() patch for Python 2.1 which was successfully used by my organization for many years. Now we are trying to move to a newer version of Python and still facing the need to patch this in. Just wondering what happened with this feature and perhaps there is something I can do to help move it along again. Many Python projects like flup, python-scgi, etc would be able to support Windows via this feature. (PS if this question is more appropriately raised on a mailing list rather than the issue tracker, I apologize in advance.) |
|
|
| msg94160 - (view) |
Author: Senthil Kumaran (orsenthil) * |
Date: 2009-10-17 01:18 |
| On Sun, Oct 04, 2009 at 05:39:26PM +0000, Preston Landers wrote: > I'm curious what happened with this issue. It says closed+accepted but > it doesn't appear to be checked in. If you see the report, the last message indicates that it is checked in revision 59004. |
|
|
| msg94257 - (view) |
Author: Preston Landers (planders) * |
Date: 2009-10-19 19:43 |
| Hmm... revision 59004 appears to be unrelated to the main issue at hand. As far as I can tell now the status is this: the dup() and fromfd() support appears to be present in Python 3000 for Windows - at least the py3k branch. I didn't check the releases. But it's not present in 2.6 or trunk. I guess as far as I'm concerned that's fine. I'm going to use a patch to 2.6.3 to implement it for my purposes anyway. Thanks. |
|
|
| msg160260 - (view) |
Author: Arthur Kantor (Arthur.Kantor) |
Date: 2012-05-09 08:34 |
| Hi guys It appears, that this patch was meant to go into both the 2.x and 3.x series, but it never made into 2.x Are there still plans to apply it to 2.7? (apologies if I'm asking this in the wrong forum) |
|
|