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.

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.

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?

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.

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

> 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.

So perhaps an int or a socket?

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?

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.

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

> 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.

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

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

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

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.

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.

Windows only supports non-blocking mode for sockets. IMO we should only support sockets on Windows. I opened the issue #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 #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.

> 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?

> 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()?

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?

> 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.

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".

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).

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.

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.

> 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.

> 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?

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

Pipes cannot be configured in non-blocking mode on Windows. It sounds dangerous to call a blocking syscall in a signal handler.

In fact, it works to write the signal number into a pipe on Windows, but I'm worried about the blocking behaviour.

> In fact, it works to write the signal number into a pipe on Windows, but I'm worried about the blocking behaviour.

It wasn't different before, so I'm not sure why we should start to worry about it?

>> In fact, it works to write the signal number into a pipe on Windows, but I'm worried about the blocking behaviour.

> It wasn't different before, so I'm not sure why we should start to worry about it?

Does you have an idea if set_wakeup_fd() is used on Windows? It's not possible to use it with select.select() because on Windows this function only accepts sockets. I don't know if it's possible to watch a pipe using IOCP. Is set_wakeup_fd() used by Twisted, Tornado or another project on Windows?

I would like to modify signal.set_wakeup_fd() to make the file descriptor (or socket) non-blocking: see issue #22042. I proposed this change to protect the user against misuse of the API, and to make the API more convinient. asyncore and asyncio modules also make files and sockets non-blocking: asyncore.dispatcher(sock) and asyncio.BaseEventLoop.connect_read_pipe() for example.

Oh, by the way, sock_xxx() methods of asyncio.BaseEventLoop don't make the socket non-blocking, and the documentation doesn't require that sockets are already set to non-blocking mode. It looks like a bug (at least in the documentation).

New changeset 5ce01ee2a8f4 by Victor Stinner in branch 'default':
Issue #22018: Fix test_set_wakeup_fd_result(), use assertEqual() not
http://hg.python.org/cpython/rev/5ce01ee2a8f4

> Does you have an idea if set_wakeup_fd() is used on Windows? It's not possible to use it with select.select() because on Windows this function only accepts sockets. I don't know if it's possible to watch a pipe using IOCP. Is set_wakeup_fd() used by Twisted, Tornado or another project on Windows?

I checked Twisted: signal.set_wakeup_fd() is only used on POSIX, as
expected (since it doesn't work with select on Windows). But Twisted
sets a signal handler for SIGINT, SIGTERM and SIGBREAK.

> Pipes cannot be configured in non-blocking mode on Windows. It sounds dangerous to call a blocking syscall in a signal handler.
>
> In fact, it works to write the signal number into a pipe on Windows, but I'm worried about the blocking behaviour.

OK, but if someone passes a socket in blocking mode, it will be accepted.
I don't understand why, if you're worried about a blocking write, you
don't just check that the FD passed is in non-blocking mode. It's
conceptually cleaner, more direct and safer.

Also, I think there's another issue open on the tracker to check just
that, so I'd leave this check out here.

2014-07-25 9:02 GMT+02:00 Charles-François Natali <report@bugs.python.org>:
>> Pipes cannot be configured in non-blocking mode on Windows. It sounds dangerous to call a blocking syscall in a signal handler.
>>
>> In fact, it works to write the signal number into a pipe on Windows, but I'm worried about the blocking behaviour.
>
> OK, but if someone passes a socket in blocking mode, it will be accepted.
> I don't understand why, if you're worried about a blocking write, you
> don't just check that the FD passed is in non-blocking mode. It's
> conceptually cleaner, more direct and safer.
>
> Also, I think there's another issue open on the tracker to check just
> that, so I'd leave this check out here.

In the issue #22042, I would like to make automatically the file desscriptor or socket handler in non-blocking mode. The problem is that you cannot make a file descriptor in non-blocking mode on Windows.

Or maybe you disagree with the issue #22042?

> In the issue #22042, I would like to make automatically the file desscriptor or socket handler in non-blocking mode. The problem is that you cannot make a file descriptor in non-blocking mode on Windows.

I don't think we should set it non-blocking automatically, but rather
check that it's non-blocking.
The first reason I can think of is that the user passing a blocking FD
could be a sign of a bug (e.g. if the other end is in blocking mode
too), and we shouldn't silently work-around it.

Charles-François wrote:

> I don't think we should set it non-blocking automatically, but rather
check that it's non-blocking.
> The first reason I can think of is that the user passing a blocking FD
could be a sign of a bug (e.g. if the other end is in blocking mode
too), and we shouldn't silently work-around it.

I hesitate between making the file descriptor non-blocking or raise an exception if it configured in blocking mode. Since I have no experience on such question, I prefer to follow your advices :-) So raise an exception if the FD is blocking.

It doesn't answer to my complain: I don't want to support file descriptors on Windows anymore because file descriptors cannot be configured in non-blocking mode.

If we raise an error if FD and sockets are blocking, calling set_wakeup_fd() on a FD on Windows would always fail. So raising an exception because the integer is a file descriptor or raising an exception because the file descriptor is blocking leads to same result: FD are not more supported on Windows.

What do you think?

> It doesn't answer to my complain: I don't want to support file descriptors on Windows anymore because file descriptors cannot be configured in non-blocking mode.

I think it does : if an exception is raised if an FD/handler is not in
non-blocking mode, this should include Windows file descriptors,
right?

> If we raise an error if FD and sockets are blocking, calling set_wakeup_fd() on a FD on Windows would always fail. So raising an exception because the integer is a file descriptor or raising an exception because the file descriptor is blocking leads to same result: FD are not more supported on Windows.

See above, I'm not sure what you're complaining about: since file
descriptors can't be made non-blocking on Windows, trying to register
them will raise an error. And since there's another issue for this, I
don't think the check belong in this patch (although it would proably
make more sense to commit the other patch first).

New version 9 of my patch:

- signal.set_wakeup_fd() now also supports socket handles on Windows

(That's all: files are still supported on Windows.)

New changeset fbd104359ef8 by Victor Stinner in branch 'default':
Issue #22018: On Windows, signal.set_wakeup_fd() now also supports sockets.
http://hg.python.org/cpython/rev/fbd104359ef8

I pushed my latest patch. Thank you for helping me to design the API of this new feature.

Let's move to the issue #22042 to discuss if signal.set_wakeup_fd() should raise an exception if the file descriptor or socket handle is blocking. I close this issue.

New changeset 963214896b22 by Victor Stinner in branch 'default':
Issue #22018: Fix test_signal: use assertEqual() not assertIs()
http://hg.python.org/cpython/rev/963214896b22