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