bpo-37811: FreeBSD, OSX: fix poll(2) usage in sockets module #15202

akhramov · 2019-08-10T20:32:18Z

FreeBSD implementation of poll(2) restricts the timeout argument to be
either zero, or positive, or equal to INFTIM (-1).

Unless otherwise overridden, socket timeout defaults to -1. This value
is then converted to milliseconds (-1000) and used as argument to the
poll syscall. poll returns EINVAL (22), and the connection fails.

This bug was discovered during the EINTR handling testing, and the
reproduction code can be found in
https://bugs.python.org/issue23618 (see connect_eintr.py,
attached). On GNU/Linux, the example runs as expected.

This change is trivial:
If the supplied timeout value is negative, truncate it to -1.

https://bugs.python.org/issue37811

the-knights-who-say-ni · 2019-08-10T20:32:21Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Our records indicate we have not received your CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

akhramov · 2019-08-13T05:09:51Z

@vstinner I'm sorry if you're the not the person I am supposed to ping, but can you take a glance at this PR?

vstinner · 2019-08-14T12:53:32Z

Modules/socketmodule.c

@@ -789,6 +789,8 @@ internal_select(PySocketSockObject *s, int writing, _PyTime_t interval,
    ms = _PyTime_AsMilliseconds(interval, _PyTime_ROUND_CEILING);
    assert(ms <= INT_MAX);

+    ms = ms < 0 ? -1 : ms;


Please add a comment explaining why you do that.

I would prefer a regular if to make it more explicit that the value is only modified it's negative. Like:

if (ms < -1) { /* FreeBSD implementation of poll(2) restricts timeout argument to be either zero, or positive, or equal to INFTIM (-1). */ ms = -1; }

Sure thing, see the updates. I borrowed the wording from #4033 -- the author was tackling exactly the same poll(2) problem.

Also I have created a news entry. I am not sure whether there's a necessity, though.

FreeBSD implementation of poll(2) restricts the timeout argument to be either zero, or positive, or equal to INFTIM (-1). Unless otherwise overridden, socket timeout defaults to -1. This value is then converted to milliseconds (-1000) and used as argument to the poll syscall. poll returns EINVAL (22), and the connection fails. This bug was discovered during the EINTR handling testing, and the reproduction code can be found in https://bugs.python.org/issue23618 (see connect_eintr.py, attached). On GNU/Linux, the example runs as expected. This change is trivial: If the supplied timeout value is negative, truncate it to -1.

miss-islington · 2019-08-14T21:22:04Z

Thanks @akhramov for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington · 2019-08-14T21:22:04Z

Thanks @akhramov for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

bedevere-bot · 2019-08-14T21:22:16Z

GH-15289 is a backport of this pull request to the 3.7 branch.

…H-15202) FreeBSD implementation of poll(2) restricts the timeout argument to be either zero, or positive, or equal to INFTIM (-1). Unless otherwise overridden, socket timeout defaults to -1. This value is then converted to milliseconds (-1000) and used as argument to the poll syscall. poll returns EINVAL (22), and the connection fails. This bug was discovered during the EINTR handling testing, and the reproduction code can be found in https://bugs.python.org/issue23618 (see connect_eintr.py, attached). On GNU/Linux, the example runs as expected. This change is trivial: If the supplied timeout value is negative, truncate it to -1. (cherry picked from commit 2814620) Co-authored-by: Artem Khramov <akhramov@pm.me>

bedevere-bot · 2019-08-14T21:22:23Z

GH-15290 is a backport of this pull request to the 3.8 branch.

FreeBSD implementation of poll(2) restricts the timeout argument to be either zero, or positive, or equal to INFTIM (-1). Unless otherwise overridden, socket timeout defaults to -1. This value is then converted to milliseconds (-1000) and used as argument to the poll syscall. poll returns EINVAL (22), and the connection fails. This bug was discovered during the EINTR handling testing, and the reproduction code can be found in https://bugs.python.org/issue23618 (see connect_eintr.py, attached). On GNU/Linux, the example runs as expected. This change is trivial: If the supplied timeout value is negative, truncate it to -1. (cherry picked from commit 2814620) Co-authored-by: Artem Khramov <akhramov@pm.me>

bedevere-bot · 2019-08-14T22:04:29Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Windows7 SP1 3.8 has failed when building commit 123f6c4.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/208/builds/268) and take a look at the build logs.
Check if the failure is related to this commit (123f6c4) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/208/builds/268

Failed tests:

test_asyncio

Failed subtests:

test_huge_content - test.test_asyncio.test_sock_lowlevel.ProactorEventLoopTests
test_huge_content_recvinto - test.test_asyncio.test_sock_lowlevel.SelectEventLoopTests

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

389 tests OK.

10 slowest tests:

test_io: 3 min 28 sec
test_multiprocessing_spawn: 3 min 6 sec
test_tools: 2 min 49 sec
test_distutils: 2 min 20 sec
test_concurrent_futures: 1 min 49 sec
test_tokenize: 1 min 48 sec
test_venv: 1 min 29 sec
test_lib2to3: 1 min 28 sec
test_asyncio: 1 min 17 sec
test_mmap: 1 min 14 sec

1 test failed:
test_asyncio

33 tests skipped:
test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_epoll
test_fcntl test_fork1 test_gdb test_grp test_ioctl test_kqueue
test_multiprocessing_fork test_multiprocessing_forkserver test_nis
test_openpty test_ossaudiodev test_pipes test_poll test_posix
test_pty test_pwd test_readline test_resource test_spwd
test_syslog test_threadsignals test_tix test_tk test_ttk_guionly
test_wait3 test_wait4 test_xxtestfuzz test_zipfile64

1 re-run test:
test_asyncio

Total duration: 14 min 35 sec

Click to see traceback logs

Traceback (most recent call last):
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\test\test_asyncio\test_sock_lowlevel.py", line 225, in test_huge_content_recvinto
    self.loop.run_until_complete(
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\base_events.py", line 608, in run_until_complete
    return future.result()
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\test\test_asyncio\test_sock_lowlevel.py", line 218, in _basetest_huge_content_recvinto
    self.assertEqual(size, 0)
AssertionError: 410176 != 0


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\windows_events.py", line 66, in _cancel_overlapped
    self._ov.cancel()
OSError: [WinError 6] The handle is invalid
k


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\windows_events.py", line 453, in finish_recv
    return ov.getresult()
OSError: [WinError 995] The I/O operation has been aborted because of either a thread exit or an application request


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\proactor_events.py", line 765, in _loop_self_reading
    f.result()  # may raise
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\windows_events.py", line 808, in _poll
    value = callback(transferred, key, ov)
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\windows_events.py", line 457, in finish_recv
    raise ConnectionResetError(*exc.args)
ConnectionResetError: [WinError 995] The I/O operation has been aborted because of either a thread exit or an application request
k


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\proactor_events.py", line 765, in _loop_self_reading
    f.result()  # may raise
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\windows_events.py", line 808, in _poll
    value = callback(transferred, key, ov)
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\windows_events.py", line 457, in finish_recv
    raise ConnectionResetError(*exc.args)
ConnectionResetError: [WinError 995] The I/O operation has been aborted because of either a thread exit or an application request


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\windows_events.py", line 453, in finish_recv
    return ov.getresult()
OSError: [WinError 64] The specified network name is no longer available


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\windows_events.py", line 66, in _cancel_overlapped
    self._ov.cancel()
OSError: [WinError 6] The handle is invalid


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\test\test_asyncio\test_sock_lowlevel.py", line 170, in test_huge_content
    self.loop.run_until_complete(
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\base_events.py", line 608, in run_until_complete
    return future.result()
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\test\test_asyncio\test_sock_lowlevel.py", line 157, in _basetest_huge_content
    data = await self.loop.sock_recv(sock, DATA_SIZE)
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\proactor_events.py", line 690, in sock_recv
    return await self._proactor.recv(sock, n)
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\windows_events.py", line 808, in _poll
    value = callback(transferred, key, ov)
  File "C:\buildbot.python.org\3.8.kloth-win64\build\lib\asyncio\windows_events.py", line 457, in finish_recv
    raise ConnectionResetError(*exc.args)
ConnectionResetError: [WinError 64] The specified network name is no longer available

…H-15202) FreeBSD implementation of poll(2) restricts the timeout argument to be either zero, or positive, or equal to INFTIM (-1). Unless otherwise overridden, socket timeout defaults to -1. This value is then converted to milliseconds (-1000) and used as argument to the poll syscall. poll returns EINVAL (22), and the connection fails. This bug was discovered during the EINTR handling testing, and the reproduction code can be found in https://bugs.python.org/issue23618 (see connect_eintr.py, attached). On GNU/Linux, the example runs as expected. This change is trivial: If the supplied timeout value is negative, truncate it to -1.

the-knights-who-say-ni added the CLA not signed label Aug 10, 2019

bedevere-bot added the awaiting review label Aug 10, 2019

akhramov mentioned this pull request Aug 10, 2019

freebsd issue when profiling network connection benfred/py-spy#147

Closed

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Aug 13, 2019

vstinner reviewed Aug 14, 2019

View changes

akhramov force-pushed the fix-issue-37811 branch from 580c2b7 to f642cf2 Compare Aug 14, 2019

vstinner merged commit 2814620 into python:master Aug 14, 2019

bedevere-bot removed the awaiting review label Aug 14, 2019

vstinner added needs backport to 3.7 needs backport to 3.8 labels Aug 14, 2019

bedevere-bot removed the needs backport to 3.7 label Aug 14, 2019

bedevere-bot removed the needs backport to 3.8 label Aug 14, 2019

bpo-37811: FreeBSD, OSX: fix poll(2) usage in sockets module #15202

bpo-37811: FreeBSD, OSX: fix poll(2) usage in sockets module #15202

akhramov commented Aug 10, 2019 •

edited by bedevere-bot

the-knights-who-say-ni commented Aug 10, 2019

akhramov commented Aug 13, 2019

vstinner Aug 14, 2019

akhramov Aug 14, 2019 •

edited

miss-islington commented Aug 14, 2019

miss-islington commented Aug 14, 2019

bedevere-bot commented Aug 14, 2019

bedevere-bot commented Aug 14, 2019

bedevere-bot commented Aug 14, 2019

bpo-37811: FreeBSD, OSX: fix poll(2) usage in sockets module #15202

bpo-37811: FreeBSD, OSX: fix poll(2) usage in sockets module #15202

Conversation

akhramov commented Aug 10, 2019 • edited by bedevere-bot

the-knights-who-say-ni commented Aug 10, 2019

akhramov commented Aug 13, 2019

vstinner Aug 14, 2019

Choose a reason for hiding this comment

akhramov Aug 14, 2019 • edited

Choose a reason for hiding this comment

miss-islington commented Aug 14, 2019

miss-islington commented Aug 14, 2019

bedevere-bot commented Aug 14, 2019

bedevere-bot commented Aug 14, 2019

bedevere-bot commented Aug 14, 2019

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

akhramov commented Aug 10, 2019 •

edited by bedevere-bot

akhramov Aug 14, 2019 •

edited