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

Race condition with timer signaling #12473

Closed
prcamp opened this issue Aug 5, 2015 · 4 comments
Closed

Race condition with timer signaling #12473

prcamp opened this issue Aug 5, 2015 · 4 comments
Labels
regression Regression in behavior compared to a previous version

Comments

@prcamp
Copy link

prcamp commented Aug 5, 2015

On the nightly 0.4.0-dev+5612, Timer() was working as intended, but since 0.4.0-dev+6210 it is broken.

Example:
Timer(x->println("hi"), 1, 0)

@vtjnash
Copy link
Member

vtjnash commented Aug 5, 2015

the problem is in the following code:

>  620 function Timer(cb::Function, timeout::Real, repeat::Real=0.0)
 621     t = Timer(timeout, repeat)
 622     @schedule begin
 623         while isopen(t)
 624             wait(t)
 625             isopen(t) && cb(t)
 626         end
 627     end
 628     t
 629 end

since the timer is non-repeating, the isopen test fails and the callback is never called

edit: broken by fe62c2e @JeffBezanson

@vtjnash vtjnash added the regression Regression in behavior compared to a previous version label Aug 5, 2015
@lobingera
Copy link

May be the header could be changed to 'non-repeating timers are broken'.

@pao pao changed the title Timers are broken One-shot timers are broken Aug 5, 2015
@vtjnash
Copy link
Member

vtjnash commented Aug 5, 2015

May be the header could be changed to 'non-repeating timers are broken'.

perhaps. the larger issue however is that having a conditional at fe62c2e#diff-ac5d350530574f3f82c860d1b963bc0aR624 incurs a race condition between the time that the timer is signaled, the time it is closed, and the time that this watcher got scheduled for execution. it's possible to create the same issues with repeating timers, the window of opportunity for failure is just much more obvious for single-shot timers.

@pao pao changed the title One-shot timers are broken Race condition with timer signaling Aug 5, 2015
@JeffBezanson
Copy link
Member

I think the best fix for now is to remove that extra condition, and call notify_error in close instead of notify. For example this is also how sockets notify readers when close is called.

vtjnash added a commit that referenced this issue Aug 10, 2015
vtjnash added a commit that referenced this issue Aug 14, 2015
ref #12473, avoid swallowing other Timer callback errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

4 participants