Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macOS 12 poll syscall returns prematurely #72274

Closed
MicroTransactionsMatterToo mannequin opened this issue Sep 11, 2016 · 34 comments
Closed

macOS 12 poll syscall returns prematurely #72274

MicroTransactionsMatterToo mannequin opened this issue Sep 11, 2016 · 34 comments
Labels
3.7 (EOL) end of life OS-mac tests Tests in the Lib/test dir

Comments

@MicroTransactionsMatterToo
Copy link
Mannequin

BPO 28087
Nosy @gpshead, @ronaldoussoren, @ncoghlan, @vstinner, @rbtcollins, @ned-deily, @MicroTransactionsMatterToo
PRs
  • bpo-28087: Skip test_asyncore and test_eintr poll failures on macOS. #462
  • [3.6] bpo-28087: Skip test_asyncore and test_eintr poll failures on macOS #463
  • bpo-28087: Skip test_asyncore and test_eintr poll failures on macOS. … #973
  • [3.5] bpo-28087: Skip test_asyncore and test_eintr poll failures on macOS #1424
  • bpo-28087: Detect broken poll() on macOS Sierra #1426
  • bpo-28087: Reenable poll() tests on macOS #1664
  • Files
  • buildlog.log: Build Log
  • eintr_tester.diff
  • issue28087.patch
  • smime.p7s
  • smime.p7s
  • 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 2020-03-18.15:16:09.677>
    created_at = <Date 2016-09-11.23:36:48.835>
    labels = ['OS-mac', '3.7', 'tests']
    title = 'macOS 12 poll syscall returns prematurely'
    updated_at = <Date 2020-03-18.15:16:09.676>
    user = 'https://github.com/MicroTransactionsMatterToo'

    bugs.python.org fields:

    activity = <Date 2020-03-18.15:16:09.676>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-03-18.15:16:09.677>
    closer = 'vstinner'
    components = ['macOS', 'Tests']
    creation = <Date 2016-09-11.23:36:48.835>
    creator = 'MicroTransactionsMatterToo'
    dependencies = []
    files = ['44569', '44572', '44575', '44576', '44577']
    hgrepos = []
    issue_num = 28087
    keywords = ['patch']
    message_count = 34.0
    messages = ['275887', '275889', '275894', '275912', '275913', '275914', '275915', '275929', '275932', '278623', '288055', '288070', '290300', '290301', '291065', '291078', '291082', '291137', '292869', '292870', '292875', '292876', '293168', '293169', '293176', '293186', '293944', '293951', '294269', '294277', '294304', '294309', '300128', '364527']
    nosy_count = 7.0
    nosy_names = ['gregory.p.smith', 'ronaldoussoren', 'ncoghlan', 'vstinner', 'rbcollins', 'ned.deily', 'MicroTransactionsMatterToo']
    pr_nums = ['462', '463', '973', '1424', '1426', '1664']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue28087'
    versions = ['Python 2.7', 'Python 3.6', 'Python 3.7']

    @MicroTransactionsMatterToo
    Copy link
    Mannequin Author

    Upon calling the select.poll objects poll method with no file descriptors registered, it will return an empty list immediately, regardless of the timeout given. This is a problem in the Mac OS X kernels implementation of the poll syscall, but the test can be easily fixed by binding a throwaway socket to the poll objects, and then using the poll method as usual.

    Build details are attached

    @MicroTransactionsMatterToo MicroTransactionsMatterToo mannequin added build The build process and cross-build OS-mac labels Sep 11, 2016
    @MicroTransactionsMatterToo
    Copy link
    Mannequin Author

    Patch exemplifying a fix for this in the test files

    @MicroTransactionsMatterToo
    Copy link
    Mannequin Author

    Fixed patch file, now in diff format

    @ned-deily
    Copy link
    Member

    Thanks for the report. This seems to be a problem visible in the public betas of the upcoming macOS 12 release, not in previous releases (10.11 and earlier). Do you know if the problem has been reported to Apple? Also, I've uploaded a reviewable version of your patch.

    @ned-deily ned-deily added 3.7 (EOL) end of life tests Tests in the Lib/test dir labels Sep 12, 2016
    @ned-deily ned-deily changed the title Mac OS X poll syscall returns prematurely macOS 12 poll syscall returns prematurely Sep 12, 2016
    @ned-deily ned-deily removed the build The build process and cross-build label Sep 12, 2016
    @MicroTransactionsMatterToo
    Copy link
    Mannequin Author

    Apple doesn’t allow access to other peoples bug reports, so I couldn’t say whether it’s been reported.

    Many Thanks,
    Ennis Massey

    Wet Ferret Studios <http://wet-ferret-studios.azurewebsites.net/\>

    @ned-deily
    Copy link
    Member

    OK, if you get a chance, it would be helpful to submit a RADAR on it and update this issue. I neglected to mention that your patch does seem to work, e.g., test_eintr no longer fails.

    @MicroTransactionsMatterToo
    Copy link
    Mannequin Author

    Ok, cool

    Many Thanks,
    Ennis Massey

    Wet Ferret Studios <http://wet-ferret-studios.azurewebsites.net/\>

    @rbtcollins
    Copy link
    Member

    @ned - any objection to my committing this at this point?

    @ned-deily
    Copy link
    Member

    I'm not sure it should be a permanent fix. Perhaps you could add the test decorator so that it is only tested on 10.12. I don't have time to look at it more myself before b1.

    @ned-deily
    Copy link
    Member

    (From curl/curl#1057, the curl project has also seen this and opened an issue with Apple against macOS 10.12, RADAR 28372390.)

    @ncoghlan
    Copy link
    Contributor

    The failures in test_eintr and test_asyncio are confusing new contributors on Mac OS X - could we get some variant of this workaround merged until Apple fix their bug?

    @ncoghlan
    Copy link
    Contributor

    Sorry, I meant test_poll and test_asyncore as described in http://bugs.python.org/issue28456 (I misremembered due to the test_poll failure mentioning EINTR)

    @ned-deily
    Copy link
    Member

    New changeset 1d391f9 by Ned Deily in branch '3.6':
    [3.6] bpo-28087: Skip test_asyncore and test_eintr poll failures on macOS (#463)
    1d391f9

    @ned-deily
    Copy link
    Member

    New changeset de04644 by Ned Deily in branch 'master':
    bpo-28087: Skip test_asyncore and test_eintr poll failures on macOS. (#462)
    de04644

    @vstinner
    Copy link
    Member

    vstinner commented Apr 3, 2017

    test_asyncore also fails on Python 2.7 on macOS Sierra. So I proposed a backport: see my PR 973.

    ======================================================================
    FAIL: test_handle_expt (test.test_asyncore.TestAPI_UsePoll)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/Users/buildbot/buildarea/2.7.billenstein-sierra/build/Lib/test/test_asyncore.py", line 620, in test_handle_expt
        self.loop_waiting_for_flag(client)
      File "/Users/buildbot/buildarea/2.7.billenstein-sierra/build/Lib/test/test_asyncore.py", line 519, in loop_waiting_for_flag
        self.fail("flag not set")
    AssertionError: flag not set

    http://buildbot.python.org/all/builders/x86-64%20Sierra%202.7/builds/2/steps/test/logs/stdio

    @vstinner
    Copy link
    Member

    vstinner commented Apr 3, 2017

    New changeset 23d6eb6 by Victor Stinner in branch '2.7':
    bpo-28087: Skip test_asyncore and test_eintr poll failures on macOS. (#462) (#973)
    23d6eb6

    @vstinner
    Copy link
    Member

    vstinner commented Apr 3, 2017

    Ah! Python 2.7 tests succeeded on Sierra!

    http://buildbot.python.org/all/builders/x86-64%20Sierra%202.7/builds/3

    @MicroTransactionsMatterToo
    Copy link
    Mannequin Author

    Makes sense it would also fail. They both use the same syscall. Gud job on the backport

    Sent from my iPhone

    On 4/04/2017, at 4:50 AM, STINNER Victor <report@bugs.python.org> wrote:

    STINNER Victor added the comment:

    Ah! Python 2.7 tests succeeded on Sierra!

    http://buildbot.python.org/all/builders/x86-64%20Sierra%202.7/builds/3

    ----------


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue28087\>


    @vstinner
    Copy link
    Member

    vstinner commented May 3, 2017

    Instead of skipping the test, it would be safer to not provide select.poll() if it's known to be broken. Curl added a check in configure:
    curl/curl@9297ca4

    The problem is that I understood that Python provides a single binary for all supported macOS versions. So if we go for a check, it should be done as runtime, as the current select_have_broken_poll() test written for macOS.

    On macOS without the bug, curl check takes 0.5 second which is not ideal :-/

    Would it be possible to design a test which doesn't block?

    @vstinner
    Copy link
    Member

    vstinner commented May 3, 2017

    Would it be possible to design a test which doesn't block?

    If there is no obvious way to detect poll() functionnaly, a workaround is to get the Darwin version from uname() and blacklist macOS versions known to have the bug (disable select.poll() on these versions).

    @vstinner
    Copy link
    Member

    vstinner commented May 3, 2017

    #1426 implements the blacklist option.

    I chose the conservative approach: hope that poll() will be fixed in 16.6.x. The problem of this approach is that test_asyncore and test_eintr will fail again if Apple doesn't fix poll().

    Since I would like to backport this fix in 2.7, 3.5 and 3.6 and not have to modify the code at each broken macOS release, maybe we can use the pessimistic solution: blacklist Darwin >= 16.x, and only start again to whitelist once Apple releases a fixed macOS.

    @vstinner
    Copy link
    Member

    vstinner commented May 3, 2017

    New changeset 23b312b by Victor Stinner in branch '3.5':
    [3.6] bpo-28087: Skip test_asyncore and test_eintr poll failures on macOS (#463) (bpo-1424)
    23b312b

    @gpshead
    Copy link
    Member

    gpshead commented May 6, 2017

    Please DO NOT REMOVE select.poll() on MacOS. It is still useful.

    Apple needs to live with their bug that they refuse to acknowledge by forcing developers to learn the special case that it now breaks in (0 fds). We should not pretend that poll() does not exist as a result.

    For reference about the details of the MacOS change they were stupid enough to force upon the world, see this comment - which links to the source of their bug in the "open" source side of their OS.
    curl/curl#1057 (comment)

    @gpshead
    Copy link
    Member

    gpshead commented May 6, 2017

    The only thing CPython should do is apply workarounds to any standard library internal uses of poll with 0 fds guarded by a MacOS platform check.

    @gpshead
    Copy link
    Member

    gpshead commented May 7, 2017

    Do we need to work around this issue at all?

    Apple appears to have fixed it already:
    opensource-apple/xnu@0cccba1#diff-e61c2932bb9d5cea2dd0732acd8ec626R1783

    A comment in curl/curl#1057 suggests Apple has shipped that change in 10.12.2.

    If you still want to have Python deal with the 10.12.0 and 10.12.1 cases you could update your PR to check those versions, but given these are security fix releases as 10.12.4 is already current I think it is better to tell people just to apply their security updates and not bother with a workaround in Python.

    @vstinner
    Copy link
    Member

    vstinner commented May 7, 2017

    You want to get poll() on macOS. I'm ok with that but I would like to see
    tests for it. We should remove the skip in the test.

    Ok, I will update my PR to just blacklist macOS 10.12.0 and 10.2.1, and
    reenable the test. I just have to find the Darwin versions.

    I don't think that we have to suggests Python users to upgrade, I expect
    macOS popups requesting to apply upgrades are enough ;-)

    @ned-deily
    Copy link
    Member

    If the problem is fixed in 10.12.2, I agree with gps that there is no need to add a workaround for 10.12.0 and .1. We should only ever need to support the most recent macOS point release; it is the user's responsibility to keep up-to-date and with most users these days that happens automatically.

    @vstinner
    Copy link
    Member

    I abandonned PR 1426 which proposed to blacklist bogus macOS versions, and instead I proposed a new PR to simply reenable previsouly skipped tests (because of the bug which is now fixed).
    #1664

    I'm unable to test my own PR right now. Ned Deily: would you mind to test it and review my change please?

    @ned-deily
    Copy link
    Member

    Unfortunately, with the tests reenabled, they still fail with the most recent release of macOS 10.12 (10.12,5) so perhaps the curl issue was a different problem?

    @vstinner
    Copy link
    Member

    Ned Deily added the comment:

    Unfortunately, with the tests reenabled, they still fail with the most recent release of macOS 10.12 (10.12,5)

    Oh. In that case, I will update my patch to blacklist macOS versions
    with the bug (remove select.poll() depending on macOS version).

    @MicroTransactionsMatterToo
    Copy link
    Mannequin Author

    Actually, having managed to get macOS to update, it seems to be odd, as it correctly delays. Not sure what’s going on there, or whether it’s my setup. I’ll look into it some more. I’m also thinking of writing/finding a set of tests for core POSIX stuff like this, so we can avoid Apple screwing stuff up like they have.
    Many Thanks,
    Ennis Massey
    ennisbaradine@gmail.com

    @MicroTransactionsMatterToo
    Copy link
    Mannequin Author

    Well, the tests worked on macOS 10.12.6 beta, although my school network broke the urllib tests
    Many Thanks,
    Ennis Massey
    ennisbaradine@gmail.com

    @vstinner
    Copy link
    Member

    What is the status of this issue? Is there still something to do?

    If you ask me my opinion, I would just suggest to remove select.poll() on macOS to stop to have to bother with poll() bugs which only trigger at runtime :-/

    @vstinner
    Copy link
    Member

    No update since 2017, I close the issue.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life OS-mac tests Tests in the Lib/test dir
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants