[PATCH v3] timer: Fix a race condition between timer's callback and destroying code

[Roman Kiryanov <rkir@google.com>]

`timerlist_run_timers` provides no mechanism to
make sure the data pointed by `opaque` is valid
when calling timer's callback: there could be
another thread running which is destroying
timer's opaque data.

With this change `timer_del` becomes blocking if
timer's callback is running and it will be safe
to destroy timer's data once `timer_del` returns.

Signed-off-by: Roman Kiryanov <rkir@google.com>
---
v2: rebased to the right branch and removed
    Google specific tags from the commit message.

v3: if a timer's callback happens to be running
    (cb_running) wait until all timers are done.
    qatomic_read/qemu_event_reset could be racing
    and timer_del might wait one extra loop of
    timers to be done.

 include/qemu/timer.h |  1 +
 util/qemu-timer.c    | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 5ce83c7911..c2c98f79f4 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -89,6 +89,7 @@ struct QEMUTimer {
     QEMUTimer *next;
     int attributes;
     int scale;
+    bool cb_running;
 };
 
 extern QEMUTimerListGroup main_loop_tlg;
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 213114be68..5ec379dc43 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -370,6 +370,7 @@ void timer_init_full(QEMUTimer *ts,
     ts->scale = scale;
     ts->attributes = attributes;
     ts->expire_time = -1;
+    ts->cb_running = false;
 }
 
 void timer_deinit(QEMUTimer *ts)
@@ -435,6 +436,10 @@ void timer_del(QEMUTimer *ts)
         qemu_mutex_lock(&timer_list->active_timers_lock);
         timer_del_locked(timer_list, ts);
         qemu_mutex_unlock(&timer_list->active_timers_lock);
+
+        if (qatomic_read(&ts->cb_running)) {
+            qemu_event_wait(&timer_list->timers_done_ev);
+        }
     }
 }
 
@@ -571,9 +576,15 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
         cb = ts->cb;
         opaque = ts->opaque;
 
+        /* prevent timer_del from returning while cb(opaque)
+         * is still running (by waiting for timers_done_ev).
+         */
+        qatomic_set(&ts->cb_running, true);
+
         /* run the callback (the timer list can be modified) */
         qemu_mutex_unlock(&timer_list->active_timers_lock);
         cb(opaque);
+        qatomic_set(&ts->cb_running, false);
         qemu_mutex_lock(&timer_list->active_timers_lock);
 
         progress = true;
-- 
2.45.2.741.gdbec12cfda-goog