classification
Title: signal.set_wakeup_fd(fd): raise an exception if the fd is in blocking mode
Type: Stage:
Components: Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: neologix, pitrou, python-dev, vstinner
Priority: normal Keywords: patch

Created on 2014-07-23 00:49 by vstinner, last changed 2014-08-27 12:02 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
signal_nonblock.patch vstinner, 2014-07-23 00:49 review
signal_check_nonblocking.patch vstinner, 2014-07-27 14:10
signal_check_nonblocking-2.patch vstinner, 2014-07-29 22:22 review
Messages (10)
msg223710 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-23 00:49
I think that signal.set_wakeup_fd(fd) must set the file descriptor to the non-blocking mode, instead of requiring the file descriptor mode to be non-blocking.

Atttached patch implements this idea.

See also the issue #22018 which proposes to support sockets in signal.set_wakeup_fd(fd) on Windows.

We have to decide if signal.set_wakeup_fd() is supposed to support files or not on Windows. Non-blocking mode does not exist for files on Windows, only for sockets.

I didn't test my patch on Windows yet.

Note: the new _Py_set_blocking() function tries to use ioctl() instead of fnctl() when available to only use one syscall instead of two. I used the same optimization in _Py_set_inheritable().
msg223712 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-07-23 01:25
I think it would be much better to expose a generic function to make a fd non-blocking, rather than bake it inside signal.set_wakeup_fd().

Also, given set_wakeup_fd() is rather specialized and uncommon, I don't see much point in making it more convenient.
msg223717 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-23 02:15
"Also, given set_wakeup_fd() is rather specialized and uncommon, I don't see much point in making it more convenient."

The problem is that passing a blocking file descriptor can block the whole Python process. The hang does not occur immediatly, but only when the pipe is full for example, which may only occur in some corner cases (ex: heavy system load, slow network, etc.).

The minimum would be to raise an error if the file descriptor is in blocking mode.
msg223777 - (view) Author: Roundup Robot (python-dev) Date: 2014-07-23 21:00
New changeset 9dc66b3a1d0d by Victor Stinner in branch 'default':
Issue #22042: Avoid dangerous C cast in socket.setblocking()
http://hg.python.org/cpython/rev/9dc66b3a1d0d
msg223785 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-23 21:41
As suggested by Antoine, I created the issue #22054 to add os.get_blocking() and os.set_blocking() functions.

Here is the simplified patch for signal.set_wakeup_fd(fd). It now depends on the patch of the issue #22054.
msg224132 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-27 14:10
signal_check_nonblocking.patch: signal.set_wakeup_fd() now raises a ValueError if the file descriptor is in blocking mode.

The patch depends on the patch of the issue #22054.

I didn't test the patch on Windows, but all calls to signal.set_wakeup_fd() must fail with ValueError since it's not possible to set a file in non-blocking mode. See the issue #22018 which proposes to support sockets for signal.set_wakeup_fd().
msg224263 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-29 22:08
On Windows, it looks like it's not possible to test if a socket handle (int, not a socket object) is blocking or not. The WSAIsBlocking() function was removed, it's only possible to set the flag using ioctlsocket().
msg224264 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-29 22:22
I commited my patch to support sockets in signal.set_wakeup_fd() on Windows.

I updated my patch. It doesn't change signal.set_wakeup_fd() on Windows anymore. It only raises an exception on POSIX if the file descriptor is blocking.

On Windows, it's not possible to make a file non-blocking and so the signal handler may block on writing in the file. Antoine Pitrou doesn't want to change the behaviour, he prefers to still support files even if there is the possible hang.

On Windows, it's not possible to check if a socket handle is blocking or not. So set_wakeup_fd() doesn't check if the socket is blocking or not on Windows.

At least, the check can be implemented on POSIX.

--

An alternative is to make the file descriptor non-blocking in set_wakeup_fd(). In this case, I suggest to drop support of files on Windows because files cannot be set in non-blocking mode.
msg225971 - (view) Author: Roundup Robot (python-dev) Date: 2014-08-27 10:59
New changeset d984dfe8c34e by Victor Stinner in branch 'default':
Issue #22042: signal.set_wakeup_fd(fd) now raises an exception if the file
http://hg.python.org/cpython/rev/d984dfe8c34e
msg225977 - (view) Author: Roundup Robot (python-dev) Date: 2014-08-27 12:02
New changeset f5f5553f219e by Victor Stinner in branch 'default':
Issue #22042: Fix test_signal on Windows
http://hg.python.org/cpython/rev/f5f5553f219e
History
Date User Action Args
2014-08-27 12:02:46python-devsetmessages: + msg225977
2014-08-27 11:00:31vstinnersetstatus: open -> closed
resolution: fixed
2014-08-27 10:59:55python-devsetmessages: + msg225971
2014-07-29 22:22:45vstinnersetfiles: + signal_check_nonblocking-2.patch

messages: + msg224264
2014-07-29 22:08:15vstinnersetmessages: + msg224263
2014-07-27 14:10:38vstinnersetfiles: + signal_check_nonblocking.patch

messages: + msg224132
2014-07-27 13:50:11vstinnersetfiles: - os_blocking-2.patch
2014-07-27 13:39:47vstinnersettitle: signal.set_wakeup_fd(fd): set the fd to non-blocking mode -> signal.set_wakeup_fd(fd): raise an exception if the fd is in blocking mode
2014-07-23 21:41:24vstinnersetfiles: + os_blocking-2.patch

messages: + msg223785
2014-07-23 21:00:07python-devsetnosy: + python-dev
messages: + msg223777
2014-07-23 02:15:19vstinnersetmessages: + msg223717
2014-07-23 01:25:22pitrousetnosy: + pitrou
messages: + msg223712
2014-07-23 00:49:37vstinnersetnosy: + neologix
2014-07-23 00:49:30vstinnercreate