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

Make test_mailbox deterministic #56076

Closed
bitdancer opened this issue Apr 18, 2011 · 13 comments
Closed

Make test_mailbox deterministic #56076

bitdancer opened this issue Apr 18, 2011 · 13 comments
Labels
performance Performance or resource usage tests Tests in the Lib/test dir

Comments

@bitdancer
Copy link
Member

BPO 11867
Nosy @pitrou, @bitdancer
Files
  • mailbox_fork_with_ipc.patch
  • test_mailbox_evt.diff
  • 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 2011-12-19.12:34:52.013>
    created_at = <Date 2011-04-18.13:19:04.078>
    labels = ['tests', 'performance']
    title = 'Make test_mailbox deterministic'
    updated_at = <Date 2011-12-20.10:50:02.956>
    user = 'https://github.com/bitdancer'

    bugs.python.org fields:

    activity = <Date 2011-12-20.10:50:02.956>
    actor = 'python-dev'
    assignee = 'none'
    closed = True
    closed_date = <Date 2011-12-19.12:34:52.013>
    closer = 'neologix'
    components = ['Tests']
    creation = <Date 2011-04-18.13:19:04.078>
    creator = 'r.david.murray'
    dependencies = []
    files = ['21705', '24049']
    hgrepos = []
    issue_num = 11867
    keywords = ['patch', 'buildbot']
    message_count = 13.0
    messages = ['133967', '134053', '134056', '134061', '134072', '134138', '149745', '149814', '149825', '149827', '149843', '149851', '149910']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'r.david.murray', 'neologix', 'sdaoden', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'resource usage'
    url = 'https://bugs.python.org/issue11867'
    versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

    @bitdancer
    Copy link
    Member Author

    Attached is a patch to remove the last sleeps from test_mailbox. I believe this makes the test suite deterministic. It also shaves 4 seconds of fixed overhead off the test run time.

    @bitdancer bitdancer added tests Tests in the Lib/test dir performance Performance or resource usage labels Apr 18, 2011
    @sdaoden
    Copy link
    Mannequin

    sdaoden mannequin commented Apr 19, 2011

    Nice ping pong you play..
    I, buildbot get:

    Using random seed 2215045
    [1/1] test_mailbox
    test test_mailbox failed -- multiple errors occurred; run in verbose mode for details
    test test_mailbox failed -- Traceback (most recent call last):
      File "/Users/steffen/usr/opt/py3k/lib/python3.3/test/test_mailbox.py", line 1033, in test_lock_conflict
        ipc.signal('done')
      File "/Users/steffen/usr/opt/py3k/lib/python3.3/test/test_mailbox.py", line 1007, in __exit__
        self.c_sock.shutdown(socket.SHUT_RDWR)
    socket.error: [Errno 57] Socket is not connected
    
    test test_mailbox failed -- Traceback (most recent call last):
      File "/Users/steffen/usr/opt/py3k/lib/python3.3/test/test_mailbox.py", line 1033, in test_lock_conflict
        ipc.signal('done')
      File "/Users/steffen/usr/opt/py3k/lib/python3.3/test/test_mailbox.py", line 1007, in __exit__
        self.c_sock.shutdown(socket.SHUT_RDWR)
    socket.error: [Errno 57] Socket is not connected

    /Users/steffen/usr/opt/py3k/lib/python3.3/test/regrtest.py:1082: ResourceWarning: unclosed <socket.socket object, fd=5, family=2, type=1, proto=0>
    gc.collect()
    1 test failed:
    test_mailbox
    Re-running failed tests in verbose mode
    Re-running test 'test_mailbox' in verbose mode

    ...

    ======================================================================
    ERROR: test_lock_conflict (test.test_mailbox.TestMbox)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/Users/steffen/usr/opt/py3k/lib/python3.3/test/test_mailbox.py", line 1033, in test_lock_conflict
        ipc.signal('done')
      File "/Users/steffen/usr/opt/py3k/lib/python3.3/test/test_mailbox.py", line 1007, in __exit__
        self.c_sock.shutdown(socket.SHUT_RDWR)
    socket.error: [Errno 57] Socket is not connected

    ======================================================================
    ERROR: test_lock_conflict (test.test_mailbox.TestMMDF)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/Users/steffen/usr/opt/py3k/lib/python3.3/test/test_mailbox.py", line 1033, in test_lock_conflict
        ipc.signal('done')
      File "/Users/steffen/usr/opt/py3k/lib/python3.3/test/test_mailbox.py", line 1007, in __exit__
        self.c_sock.shutdown(socket.SHUT_RDWR)
    socket.error: [Errno 57] Socket is not connected

    Ran 332 tests in 55.487s

    FAILED (errors=2)
    test test_mailbox failed -- multiple errors occurred
    ok

    ... repeats yet another two times (very long output though)

    ----------------------------------------------------------------------
    Ran 332 tests in 151.484s

    OK
    [118424 refs]

    @bitdancer
    Copy link
    Member Author

    Thanks for testing this. I was afraid something like that would happen, since socket implementations are different on different platforms. I presume you ran it on OSX.

    Now I have to decide if I want to fix it, or if I should just switch to using threads.

    @sdaoden
    Copy link
    Mannequin

    sdaoden mannequin commented Apr 19, 2011

    Short glance into it, and after
    commenting out
    self.c_sock_close = self.c_sock_shutdown = True
    in the parent process it ends up as
    (maybe i can spend more time this evening):

    15:36 ~/tmp $ python3 -E -Wd -m test -r -w -uall test_mailbox
    Using random seed 9754656
    [1/1] test_mailbox
    /Users/steffen/usr/opt/py3k/lib/python3.3/mailbox.py:723: ResourceWarning: unclosed <socket.socket object, fd=5, family=2, type=1, proto=0>
    return self._toc[key]
    /Users/steffen/usr/opt/py3k/lib/python3.3/mailbox.py:77: ResourceWarning: unclosed <socket.socket object, fd=5, family=2, type=1, proto=0>
    return self.get_message(key)
    1 test OK.
    [118589 refs]

    @bitdancer
    Copy link
    Member Author

    I think the fix is to either put a try/except around the socket shutdown call, or to remove it entirely (I think things will still work right on linux without it). If you leave the self.c_sock_close = True in, it should take care of the resource warning.

    @sdaoden
    Copy link
    Mannequin

    sdaoden mannequin commented Apr 20, 2011

    'Was not allowed to look yesterday.
    If the child only closes and not
    self.c_sock_shutdown = True
    (shutdown is an ugly word for a child anyway) then
    (and i hope Apple did not modify the BSD network stack):

    12:59 ~/tmp $ python3 -E -Wd -m test -r -w test_mailbox
    Using random seed 1985762
    [1/1] test_mailbox
    1 test OK.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Dec 18, 2011

    There was a recent buildbot failure on test_lock_conflict() because of a race.
    Looking at your patch, I must be missing something, but why not simply use a multiprocessing condition to signal when the parent process has acquired the lock?
    Otherwise you could simply use a pipe or a socketpair, it would be much simpler.
    Also, there's a race: if the parent process calls connect() before the child calls listen(), he'll get ECONNREFUSED.

    @bitdancer
    Copy link
    Member Author

    Probably because I'm a threading/multiprocessing neophyte :) (Though I just learned a bunch about programming with threads recently...)

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Dec 19, 2011

    Probably because I'm a threading/multiprocessing neophyte :)

    That's a very good reason :-)

    Here's a version using two multiprocessing events. Note that I use
    timeouts for wait() just to avoid being stuck if something goes wrong:
    the test now runs in a dozen ms.

    By the way, next time you need a duplex communication between two
    processes, you can use socketpair(), which returns a pair of connected
    sockets.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 19, 2011

    Charles-François's patch looks good to me.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 19, 2011

    New changeset c6d41dd60d2d by Charles-François Natali in branch '2.7':
    Issue bpo-11867: Make test_mailbox.test_lock_conflict deterministic (and fix a
    http://hg.python.org/cpython/rev/c6d41dd60d2d

    New changeset 0053b7c68a02 by Charles-François Natali in branch '3.2':
    Issue bpo-11867: Make test_mailbox.test_lock_conflict deterministic (and fix a
    http://hg.python.org/cpython/rev/0053b7c68a02

    New changeset 020260ec44f2 by Charles-François Natali in branch 'default':
    Issue bpo-11867: Make test_mailbox.test_lock_conflict deterministic (and fix a
    http://hg.python.org/cpython/rev/020260ec44f2

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Dec 19, 2011

    Should be fixed now, thanks!

    @neologix neologix mannequin closed this as completed Dec 19, 2011
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 20, 2011

    New changeset c9facd251725 by Charles-François Natali in branch '2.7':
    Followup to issue bpo-11867: Use socketpair(), since FreeBSD < 8 doesn't really
    http://hg.python.org/cpython/rev/c9facd251725

    New changeset 9dee8a095faf by Charles-François Natali in branch '3.2':
    Followup to issue bpo-11867: Use socketpair(), since FreeBSD < 8 doesn't really
    http://hg.python.org/cpython/rev/9dee8a095faf

    New changeset 9014e0cc5589 by Charles-François Natali in branch 'default':
    Followup to issue bpo-11867: Use socketpair(), since FreeBSD < 8 doesn't really
    http://hg.python.org/cpython/rev/9014e0cc5589

    @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
    performance Performance or resource usage tests Tests in the Lib/test dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants