Small improvements to Timer (#39510)

* delay launching Task until just before it is needed

This is a performance/memory optimization: previously, it would start
running at an indeterminate time between when it was defined and when it
was needed, possibly wasting memory until then.

* Timer: don't create a one-shot timer

* Timer: Don't fail silently when callback fails

Co-authored-by: Jameson Nash <jameson@juliacomputing.com>

base/asyncevent.jl

@@ -43,10 +43,13 @@ the async condition object itself.
 """
 function AsyncCondition(cb::Function)
     async = AsyncCondition()
-    @async while _trywait(async)
-            cb(async)
-            isopen(async) || return
-        end
+    t = @task while _trywait(async)
+        cb(async)
+        isopen(async) || return
+    end
+    lock(async.cond)
+    _wait2(async.cond, t)
+    unlock(async.cond)
     return async
 end
 
@@ -72,7 +75,7 @@ mutable struct Timer
         timeout ≥ 0 || throw(ArgumentError("timer cannot have negative timeout of $timeout seconds"))
         interval ≥ 0 || throw(ArgumentError("timer cannot have negative repeat interval of $interval seconds"))
         timeout = UInt64(round(timeout * 1000)) + 1
-        interval = UInt64(round(interval * 1000))
+        interval = UInt64(ceil(interval * 1000))
         loop = eventloop()
 
         this = new(Libc.malloc(_sizeof_uv_timer), ThreadSynchronizer(), true, false)
@@ -248,10 +251,19 @@ julia> begin
 """
 function Timer(cb::Function, timeout::Real; interval::Real=0.0)
     timer = Timer(timeout, interval=interval)
-    @async while _trywait(timer)
+    t = @task while _trywait(timer)
+        try
             cb(timer)
-            isopen(timer) || return
+        catch err
+            write(stderr, "Error in Timer:\n")
+            showerror(stderr, err, catch_backtrace())
+            return
         end
+        isopen(timer) || return
+    end
+    lock(timer.cond)
+    _wait2(timer.cond, t)
+    unlock(timer.cond)
     return timer
 end
 

base/condition.jl

@@ -78,6 +78,14 @@ islocked(c::GenericCondition) = islocked(c.lock)
 lock(f, c::GenericCondition) = lock(f, c.lock)
 unlock(f, c::GenericCondition) = unlock(f, c.lock)
 
+# have waiter wait for c
+function _wait2(c::GenericCondition, waiter::Task)
+    ct = current_task()
+    assert_havelock(c)
+    push!(c.waitq, waiter)
+    return
+end
+
 """
     wait([x])
 
@@ -99,8 +107,7 @@ proceeding.
 """
 function wait(c::GenericCondition)
     ct = current_task()
-    assert_havelock(c)
-    push!(c.waitq, ct)
+    _wait2(c, ct)
     token = unlockall(c.lock)
     try
         return wait()

test/channels.jl

@@ -517,6 +517,13 @@ let a = []
     @test a == [1]
 end
 
+# make sure that we don't accidentally create a one-shot timer
+let
+    t = Timer(t->nothing, 10, interval=0.00001)
+    @test ccall(:uv_timer_get_repeat, UInt64, (Ptr{Cvoid},), t) == 1
+    close(t)
+end
+
 # make sure repeating timers work
 @noinline function make_unrooted_timer(a)
     t = Timer(0.0, interval = 0.1)