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

threading.Condition.wait() return value indicates timeout #41798

Closed
blais mannequin opened this issue Apr 3, 2005 · 8 comments
Closed

threading.Condition.wait() return value indicates timeout #41798

blais mannequin opened this issue Apr 3, 2005 · 8 comments
Labels
easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@blais
Copy link
Mannequin

blais mannequin commented Apr 3, 2005

BPO 1175933
Nosy @gvanrossum, @tim-one, @tiran, @benjaminp
Files
  • wait-return-value.patch: Patch to add return value to threading.Condition.wait()
  • 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 2009-03-30.15:45:36.314>
    created_at = <Date 2005-04-03.19:09:47.000>
    labels = ['easy', 'type-feature', 'library']
    title = 'threading.Condition.wait() return value indicates timeout'
    updated_at = <Date 2009-03-30.15:45:36.296>
    user = 'https://bugs.python.org/blais'

    bugs.python.org fields:

    activity = <Date 2009-03-30.15:45:36.296>
    actor = 'gvanrossum'
    assignee = 'none'
    closed = True
    closed_date = <Date 2009-03-30.15:45:36.314>
    closer = 'gvanrossum'
    components = ['Library (Lib)']
    creation = <Date 2005-04-03.19:09:47.000>
    creator = 'blais'
    dependencies = []
    files = ['6588']
    hgrepos = []
    issue_num = 1175933
    keywords = ['patch', 'easy']
    message_count = 8.0
    messages = ['48139', '48140', '48141', '48142', '60173', '63115', '63123', '84567']
    nosy_count = 7.0
    nosy_names = ['gvanrossum', 'tim.peters', 'blais', 'mdehoon', 'christian.heimes', 'benjamin.peterson', 'bgnori']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue1175933'
    versions = ['Python 2.6']

    @blais
    Copy link
    Mannequin Author

    blais mannequin commented Apr 3, 2005

    A condition variable returns in two cases: it was
    notified by another thread, or it timed out (if a
    timeout was specified). See an example in the popular
    Boost C++ library:

    http://boost.org/doc/html/condition.html

    This patch adds this capability to the Python
    threading.Condition.wait() method, which used to return
    nothing. I added the relevant couple of lines to the
    documentaion as well (see patch).

    (An example is using a condition variable as a sentinel
    for exiting a loop or a polling thread. Using the
    return value one can decide whether to exit the loop or
    not.)

    @blais blais mannequin added stdlib Python modules in the Lib dir labels Apr 3, 2005
    @mdehoon
    Copy link
    Mannequin

    mdehoon mannequin commented Apr 29, 2005

    Logged In: YES
    user_id=488897

    This looks like a good idea to me. It will help to clean up
    the "get" method in Queue.py, which now has:

                    while self._empty():
                        remaining = endtime - _time()
                        if remaining <= 0.0:
                            raise Empty
                        self.not_empty.wait(remaining)

    Here, self.not_empty is an object of the class
    threading.Condition. It seems odd that first we wait for
    self.not_empty.wait to return, and then have to check
    self._empty(), even though self.not_empty.wait could have
    told us directly if it was notified or it timed out.

    I'll write a message to python-dev in support of this patch
    (I'm a mere patch reviewer, not an official Python developer).

    @tim-one
    Copy link
    Member

    tim-one commented Apr 30, 2005

    Logged In: YES
    user_id=31435

    Sorry, I think this is a poor idea, and mdehoon's suggestion
    for turning a correct use of .wait() into a doubly buggy one
    illustrates why: there's no guarantee that self._empty() will
    return false just because .wait() returns without timing out --
    .wait() returning just means that it may not be a waste of
    time to check for the desired condition. "notifying" a condvar
    emphatically does not mean that the desired condition holds,
    and any number of other threads can run for any amount of
    time between the times a condvar is notified and some wait()
    er wakes up (so even if the desired condition does hold at the
    time notify() is called, it may not anymore by the time a wait()
    er wakes).

    The check should always be done when .wait() doesn't time
    out, and even if .wait() does time out, self._empty() may
    return false anyway.

    Note too that .wait()'s caller holds the associated mutex
    regardless of whether return is due to timeout or notify, and
    the caller needs to release it again in either case. Creating a
    distinction based on return value creates a new opportunity to
    screw up that part too.

    I don't understand this:

    An example is using a condition variable as a sentinel
    for exiting a loop or a polling thread. Using the
    return value one can decide whether to exit the loop or
    not.)

    To the extent that I might understand it, it sounds like a
    condvar is gross overkill, and that you'd want something
    simpler (perhaps an Event) in those cases. But I can't flesh
    out the code you have in mind there.

    @blais
    Copy link
    Mannequin Author

    blais mannequin commented Apr 30, 2005

    Logged In: YES
    user_id=10996

    hi tim
    thanks for the comments.

    my use of a condition variable is exemplified in this
    background thread, which computes something periodically,
    and with which the main thread uses a condition variable to
    communicate "pause" and "quit" states (there is
    communication between only those two threads):

    def background_thread( func, args, condvar, callbacks, delay ):
        """
        A background thread that computes something periodically.
    @func: function that computes something;
    @data: arguments for function that computes something;
    @condvar: a variable to indicate for the thread to stop
    

    or pause.
    The variable is the 'pause' and 'quit' members
    on the condvar.
    @callbacks: a list of functions to call when a new
    result becomes available;
    """
    done = 0
    while not done:
    # compute something
    data = func(*args)

        \# push the new data into the callbacks
        for cb in callbacks:
            cb(data)
    
            condvar.acquire()
            if condvar.pause:
                # this branch is used to pause the thread.
                condvar.wait()
            else:
                condvar.wait(delay)
            if condvar.quit:
                done = 1
            condvar.release()

    instead of using the "quit" data member, i could instead use
    the return value of the wait() call. basically, exit the
    loop when i've been signaled.

    AFAIK this is a common usage pattern. i've seen it in a few
    places. do you see any problem with this usage?

    not that it's really an argument, but i did see 3
    independent implementations of wait() returning the
    signaled/timeout boolean.

    i think Event.wait() should also return the signaled/timeout
    state. it's information that should be returned to the
    user, whether he uses it or not. sure, people can screw up,
    but if there is a legitimate case where it can be used, i
    think it should be provided anyway.

    also, you are correct about mdehoon's case, after returning
    from wait(), one should check for the queue being empty or not.

    @tiran
    Copy link
    Member

    tiran commented Jan 19, 2008

    Another bug day task.

    @tiran tiran added easy type-feature A feature request or enhancement labels Jan 19, 2008
    @bgnori
    Copy link
    Mannequin

    bgnori mannequin commented Feb 29, 2008

    I have a question:
    Why not to raise an exception?

    Timeout means "something has messed up, can not continue."
    We are not doing task scheduling, aren't we?

    @benjaminp
    Copy link
    Contributor

    I don't think an exception is the proper thing to use here. Having the
    wait timeout is not exceptional or unexpected.

    Timeout means "something has messed up, can not continue."
    Not really. It means "we didn't want to wait this long to be notified."

    @gvanrossum
    Copy link
    Member

    Since Tim Peters is the absolute authority on concurrency in Python,
    let's just close this as Rejected rather than letting it sit here forever.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants