signal.set_wakeup_fd() should accept sockets on Windows

[vstinner]

BPO	22018
Nosy	@warsaw, @terryjreedy, @pfmoore, @ronaldoussoren, @vstinner, @tjguk, @cjw296, @ned-deily, @ezio-melotti, @merwok, @bitdancer, @asvetlov, @zware, @1st1, @koobs, @zooba, @dstufft, @moreati, @lysnikolaou, @pablogsal, @ខ្មែរសប់ហាងលក់ទំនិញគ្រប់ប្រភេទ
Files	signal_socket.patch signal_socket-2.patch signal_socket-3.patch signal_socket-4.patch wakeup_socket-6.patch wakeup_fd-7.patch wakeup_fd-8.patch wakeup_fd-9.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 2014-07-29.21:49:54.567>
created_at = <Date 2014-07-20.19:47:13.298>
labels = ['expert-regex', 'expert-XML', 'build', 'tests', 'OS-mac', 'expert-tkinter', 'expert-IO', 'expert-IDLE', 'expert-asyncio', 'ctypes', 'OS-windows', 'interpreter-core', 'expert-2to3', 'expert-C-API', 'expert-unicode', 'docs', 'type-security', 'expert-SSL', 'expert-installation', 'expert-email', 'expert-subinterpreters', 'extension-modules', 'library', 'expert-argument-clinic']
title = 'signal.set_wakeup_fd() should accept sockets on Windows'
updated_at = <Date 2021-06-19.19:19:59.557>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2021-06-19.19:19:59.557>
actor = 'larry'
assignee = 'docs@python'
closed = True
closed_date = <Date 2014-07-29.21:49:54.567>
closer = 'vstinner'
components = ['Build', 'Demos and Tools', 'Distutils', 'Documentation', 'Extension Modules', 'IDLE', 'Installation', 'Interpreter Core', 'Library (Lib)', 'macOS', 'Regular Expressions', 'Tests', 'Tkinter', 'Unicode', 'Windows', 'XML', '2to3 (2.x to 3.x conversion tool)', 'ctypes', 'IO', 'Cross-Build', 'email', 'asyncio', 'Argument Clinic', 'FreeBSD', 'SSL', 'C API', 'Subinterpreters', 'Parser']
creation = <Date 2014-07-20.19:47:13.298>
creator = 'vstinner'
dependencies = []
files = ['36006', '36011', '36033', '36038', '36056', '36071', '36072', '36130']
hgrepos = []
issue_num = 22018
keywords = ['patch']
message_count = 41.0
messages = ['223532', '223533', '223534', '223537', '223538', '223539', '223543', '223548', '223551', '223574', '223575', '223578', '223580', '223667', '223708', '223709', '223792', '223798', '223822', '223831', '223852', '223870', '223872', '223880', '223884', '223886', '223892', '223897', '223898', '223905', '223908', '223911', '223924', '223926', '223940', '223942', '223977', '224130', '224256', '224259', '224292']
nosy_count = 23.0
nosy_names = ['barry', 'terry.reedy', 'paul.moore', 'ronaldoussoren', 'vstinner', 'tim.golden', 'cjw296', 'ned.deily', 'ezio.melotti', 'eric.araujo', 'mrabarnett', 'r.david.murray', 'asvetlov', 'docs@python', 'zach.ware', 'yselivanov', 'koobs', 'steve.dower', 'dstufft', 'Alex.Willmer', 'lys.nikolaou', 'pablogsal', 'habrecord22']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue22018'
versions = ['Python 3.5']

[vstinner]

Hi,

I'm working on asyncio, someone asked why asyncio cannot be interrupted by CTRL+c (SIGINT):
https://code.google.com/p/tulip/issues/detail?id=191

On Windows, select.select() is not interrupted by CTRL+c. To get a reliable behaviour, interrupt select.select() on a signal, we can use signal.set_wakeup_fd(). New problem: on Windows, select.select() only supports sockets.

I propose to modify signal.set_wakeup_fd() to not only support files (use write), but also sockets (use send). Attached patch implements that.

This issue is part of a global PEP to modify how Python handles EINTR. I'm writing a PEP on that with Charles-François Natali. The idea to retry on EINTR in some cases, like write(), so we need a way to wakeup the code on a signal, on all platforms.

Questions:

• I had to modify the pythoncore Visual Studio project to add a dependency to the WinSock library ("ws2_32.lib"). Is it a bad thing? We may split the _signal module into a new project, to only put the dependency there. What do you think? I'm not sure that it makes sense because the _signal module is always imported at Python startup, by initsigs() (search for the call to PyOS_InitInterrupts()).

• PySignal_SetWakeupFd() returns the old file descriptor, which is -1 by default. The API is not written to report errors. I chose to return -2 and clear the Python exception. Should we add a new function raising a Python exception on error? Ex: "int PySignal_SetWakeupFdWithError(int fd, int *old_fd)" returns 0 on success, -1 on error.

+ /* Import the _socket module to call WSAStartup() */
+ mod = PyImport_ImportModuleNoBlock("_socket");

I chose to import the _socket module because calling WSAStartup() requires also to call WSACleanup() at exit. I don't want to have two modules responsible for that.

I'm using "getsockopt(fd, SOL_SOCKET, SO_ERROR, ...)" to check if fd is a socket. Is there a better function to check if a file descriptor or handle is a socket?

According to the documentation, "GetFileType(handle) == FILE_TYPE_PIPE" is true for sockets, but also for (named and anonymous) pipes.

[vstinner]

I tested manually test_signal.test_socket() on Windows 7: the test pass.

The test is currently skipped on Windows because the whole TestCase is skipped. Moreover, there is no socket.socketpair() function on Windows. For the manual test, I used asyncio.windows_utils.socketpair(). I prefer to not make test_signal depends on the asyncio module.

[gvanrossum]

I don't know much about how sockets are implemented on Windows; is there a guarantee that the space of possible socket fds doesn't overlap the space of possible stream fds?

[vstinner]

Guido wrote:
"I don't know much about how sockets are implemented on Windows; is there a guarantee that the space of possible socket fds doesn't overlap the space of possible stream fds?"

Python has a low-level _PyVerify_fd() which uses the __pioinfo private array. It looks socket "handles" (small integers betwen 256 and 1000 in my quick tests) and file descriptors overlap. Files get file descriptors between 0 and 2047.

[gvanrossum]

Perhaps the API should take a socket object on Windows to avoid any confusion?

[vstinner]

Perhaps the API should take a socket object on Windows to avoid any confusion?

I don't know if the feature is useful, but signal.set_wakeup_fd() works on Windows since at least Python 2.7 and accepts files. Only supporting sockets would be a regression.

[gvanrossum]

So perhaps an int or a socket?

[vstinner]

I don't understand. My patch works for files and sockets on all platforms.
What is the problem? You don't like the idea of checking the file type?

[gvanrossum]

My worry is that somehow a program has a fd that refers to both a file and a socket. But I agree that changing the API is not a great option either.

[python-dev]

New changeset 6b536f0516ea by Victor Stinner in branch 'default':
Issue bpo-22018: Add _testcapi.raise_signal()
http://hg.python.org/cpython/rev/6b536f0516ea

[vstinner]

My worry is that somehow a program has a fd that refers to both a file and a socket. But I agree that changing the API is not a great option either.

Well, when I read again my patch and played with it, I saw that it has different issues:

• a Windows socket handle is not an int, but a pointer: Python SOCKET_T type should be used instead

• when send() fails, we should reuse the code from socketmodule.c to raise an exception: we may need to check GetLastError() on Windows

I rewrote my patch to add a new function signal.set_wakeup_socket() instead of trying to guess if the file descriptor is a socket or a file. I adopted a similar approach in the PEP-446 with os.set_inheritable() and os.set_handle_inheritable() for the same reason: support sockets on Windows, socket.socket.set_inheritable() uses os.set_inheritable() on UNIX and os.set_handle_inheritable() on Windows.

signal.set_wakeup_socket() now takes a socket.socket object and returns the previous socket object (or None).

In the new patch, signal.set_wakeup_fd() and Py_SetWakeupFd() function are unchanged, which is more safer regarding to backward compatibility!

The first call to signal.set_wakeup_socket() imports the _socket module.

The Visual Studio still needs to be modified to add the dependency to the WinSock library ("ws2_32.lib"), just for the send() function.

[python-dev]

New changeset 42cf963e3ab1 by Victor Stinner in branch 'default':
Issue bpo-22018: signal.set_wakeup_fd() now raises an OSError instead of a
http://hg.python.org/cpython/rev/42cf963e3ab1

[python-dev]

New changeset 7a1737033a23 by Victor Stinner in branch 'default':
Issue bpo-22018: Hum, set_wakeup_fd() still raises ValueError on Windows
http://hg.python.org/cpython/rev/7a1737033a23

[vstinner]

New patch to rebase the code and document the new function. It fixed also the docstring.

[vstinner]

I talked with Charles-François. He doesn't like the idea of a new function because it would be harder to write portable code: which function should be used? signal.set_wakeup_fd() already works with sockets on UNIX.

Here is a new patch version 4 which tries to combine previous patchs:

• don't touch anything to existing code on UNIX. Keep the sig_atomic_t type for wakeup_fd and continue to use write() on sockets.

• on Windows, use a new "wakeup" structure to store the file descriptor or socket handle (use the SOCKET_T type) and the last send() errors (errno and GetLastError())

• on Windows, use getsockopt(fd, SOL_SOCKET, SO_ERROR, ...) to check if fd is a socket: the function fails with WSAENOTSOCK if fd is not a socket.

• on Windows, PySignal_SetWakeupFd() only supports file descriptors. The function returns -1 if signal.set_wakeup_fd() was called with a socket handler, because the return type (int) is too small to store a socket handle (need type SOCKET_T) and we cannot change the prototype of this public API.

Reminder: the purpose of the whole patch is to be able to wakeup an event loop based on select.select() on Windows. select() only supports sockets on Windows.

Notes:

• we still need to add a dependency to the WinSock library

• the test are still skipped on Windows, I will work on a new patch to make the file descriptor or socket non-blocking and to run more wakeup fd tests on Windows

• if send() fails twice, the error is only reported once until report_wakeup_send_error() is called to consume the error. Anything, if send() failed, it will probably fail again quickly, there is no need to flood the terminal with these errors.

[vstinner]

I tried to modify signal.set_wakeup_socket() to automatically make the file descriptor non-blocking. Problem: fcntl() function and O_NONBLOCK flag don't exist on Windows. Non-blocking operations are only supported for sockets...

Calling a blocking function (write()) in a signal handler will probably block the application. So I don't think that it makes sense to support files in signal.set_wakeup_fd() on Windows. I guess that nobody used this function on Windows in fact.

I can probably simplify my patch to only support sockets on Windows.

[vstinner]

Windows only supports non-blocking mode for sockets. IMO we should only support sockets on Windows. I opened the issue bpo-22042 which proposes to change signal.set_wakeup_fd(fd) to automatically make the file descriptor non-blocking. I cannot implement it on Windows because files cannot be configured to non-blocking mode.

I have an API design issue. Choices:

(A) On UNIX, signal.set_wakeup_fd(fd) is unchanged: accept any file descriptors (int). On Windows, signal.set_wakeup_fd(fd) only accepts socket objects (socket.socket).

(B) On UNIX, signal.set_wakeup_fd(fd) is unchanged: accept any file descriptors (int). On Windows, signal.set_wakeup_fd(fd) only accepts socket handles (int).

(C) signal.set_wakeup_fd(fd) is unchanged but it is no more available on Windows (signal.set_wakeup_fd does not exist anymore, same change for PySignal_SetWakeupFd). Add a new signal.set_wakeup_socket(socket) function which only accepts socket objects (socket.socket), available on all platforms.

The issue bpo-22042 (make the file descriptor or socket automatically non-blocking) can be implemented with any of these options.

The option (A) is really ugly: only accepting int on UNIX and only socket.socket on Windows doesn't look like a portable API.

I don't like the option (B). Sockets are usually stored as objects (socket.socket) because of Windows, not as socket handle (int)

So my favorite option is (C).

On Windows, socket.fileno() returns a socket handle. Even if socket handles and file descriptors look the same (small integers), their namespace are completly separated. Operations on socket handles don't accept file descriptor. Operations on file descriptors don't accept socket handles.

Socket objects are preferred for portability. For example, socket.send() works on all platforms.

The option (C) also avoids the need of guessing the file type. On Windows, there is no function to check if a number is a file descriptor or a socket handle. _PyVerify_fd() tells you if a number if a file descriptor or not. But there is no public Windows function to check if a number is a socket handle.

The option (C) also makes the implemantion simpler: the signal module can:

• call socket.setblocking(False) to make the socket non-blocking,
• get directly the socket handle/file descriptor from the socket object (using the private C structure),
• call the socket error handler instead of copying the code.

I'm not sure that getsockopt(sock_fd, SOL_SOCKET, SO_ERROR, ...) is reliable. I had bad experiences why I designed os.get_inheritable(fd) vs os.get_handle_inheritable(handle): calling os.get_inheritable() with a handle created file descriptors, which is something really strange. I'm no more sure about that, but I remember a very strange behaviour.

Python now has os.get_inheritable(fd) for file descriptors and os.get_handle_inheritable(handle) for handles (it is only used for socket handles in factor) to not guess.

[pitrou]

I don't like the option (B). Sockets are usually stored as objects (socket.socket) because of Windows, not as socket handle (int)

I don't understand this. If you're ok with calling fileno() under Linux, why not under Windows?

[vstinner]

I don't understand this. If you're ok with calling fileno() under Linux, why not under Windows?

I propose to add set_wakeup_socket() for all platforms. This function doesn't really call the fileno() method, it gets the socket file descriptor/socket handle from the C structure.

I explained why I prefer to use an object rather than a number for set_wakeup_socket(). For example, it makes a clear separation between set_wakeup_fd(int) and set_wakeup_socket(socket).

Would you prefer to use the file descriptor/socket handler for set_wakeup_socket()?

[pitrou]

Le 24/07/2014 06:00, STINNER Victor a écrit :

STINNER Victor added the comment:

> I don't understand this. If you're ok with calling fileno() under Linux, why not under Windows?

I propose to add set_wakeup_socket() for all platforms.

That's not what I'm answering to, though. See option B above.

Again, what's wrong with passing the socket as a fileno?

[vstinner]

That's not what I'm answering to, though. See option B above.
Again, what's wrong with passing the socket as a fileno?

There is nothing "wrong", it's just that I prefer option (C) over the option (B).

Quick poll in the Python stdlib for functions accepting sockets on Windows.

Expect a socket object:

• asyncore.dispatcher.set_socket()
• ssl.wrap_socket(), ssl.SSLSocket()

Expect a (socket) handle:

• os.set_handle_inheritable(), function accepting any kind of handle, not only socket handles

Accept a file descriptor or an object with a fileno() method:

• select.select(), select.poll()

Hum, I'm not convinced by the poll :-/ There are too few functions to use it to take a decision.

On UNIX, sockets are just file descriptors, like any other file descriptor. So all functions accepting file descriptors accept sockets.

--

Note: select.select() uses "int PyObject_AsFileDescriptor(PyObject *o)" to get the socket handle of a socket, I would expect the SOCKET_T type here. Does it mean that socket handle fits in a C int? Yes according to this article:

http://stackoverflow.com/questions/1953639/is-it-safe-to-cast-socket-to-int-under-win64

"Even though sizeof(SOCKET) is 8, it's safe to cast it to int, because the value constitutes an index in per-process table of limited size and not a real pointer."

"The per-process limit on kernel handles is 2^24."

I wrote a stress test creating and closing sockets in a loop. I ran the test on Windows 7 64 bit with 1 GB of memory. The maximum seen socket handle is 1,330,836 after creating 5,613,807 sockets (with a list of 331,343 open socekts), it's much smaller than 2^32.

OpenSSL stores socket handles in C int.

[neologix]

As I said offline to Victor, I think it would be better to have a single function, i.e. keep set_wakeup_fd(). It makes the API simpler, less confusing and error prone: some people will wonder which one they should use, might end up using the wrong one, or both.
Furthermore, it makes writing portable Python code more difficult, since the user has to chose the right function.
If set_wakeup_fd() can do an fstat() (or whatever that it on Windows) to detect the FD type and call send() instead of write() on a socket, all the above issues would go away.
"Never let the user do what the library can do for him".

[gvanrossum]

I find Charles' argument pretty convincing. The whole point of this API is to have another thing you can add to the selector to deal with a race condition. You then pass this thing's fileno() to signal.set_wakeup_fd(). That should be totally portable.

I'm find with it requiring a fd that refers to a socket on Windows; plain files aren't selectable anyway (not even on UNIX).

[vstinner]

Ok, let's go with the option (B): use set_wakeup_fd() on all platforms, but only accept socket handles on Windows.

New patch wakeup_fd-7.patch:

• signal.set_wakeup_fd() now only accepts socket handles (int) on Windows, it raises TypeError for files. Note: it also raises TypeError for closed sockets (I don't think that it's possible to check if a integer is a closed file descriptor or a closed socket).

• PySignal_SetWakeupFd() uses Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int) to convert the socket handle to an int. It's safe according to msg223852.

Sorry, it took me several versions to design the API. I discovered that:

• files cannot be non-blocking on Windows (so signal.set_wakeu_fd() is almost useless on Windows in Python 3.4),

• socket handles should be stored in SOCKET_T (not int) which caused me issues with the PySignal_SetWakeupFd() prototype (result type is int),

• in fact, it's safe to cast SOCKET_T to int.

Guido wrote:

My worry is that somehow a program has a fd that refers to both a file and a socket. But I agree that changing the API is not a great option either.

I don't think that it's possible that a file descriptor and a socket handle have the same value.

[gvanrossum]

I think if it's not a socket (or a closed one) it should raise ValueError or perhaps OSError -- TypeError would mean that it's not an int.

[vstinner]

I think if it's not a socket (or a closed one) it should raise ValueError or perhaps OSError -- TypeError would mean that it's not an int.

Oh, you're right. Updated patch, version 8, now raises a ValueError.

[neologix]

Ok, let's go with the option (B): use set_wakeup_fd() on all platforms, but only accept socket handles on Windows.

Sorry, why restrict it to sockets on Windows?
If someone wants to pass e.g. a pipe, why prevent it?