Currently there is no mechanism guaranteeing that it is safe to delete the object pointed by opaque in timer_init.
This race condition happens if a timer is created on a separate thread and timer_del is called between qemu_mutex_unlock and cb(opaque) in timerlist_run_timers. In this case the user thread will continue executing (once timer_del returns) which could cause destruction of the object pointed by opaque while the callback is (or will be) using it. See the example below: void myThreadFunc() { void *opaque = malloc(42); QEMUTimer timer; timer_init(&timer, myClockType, myScale, &myTimerCallbackFunc, opaque); ... timer_del(&timer); free(opaque); } This change adds a function, timer_join, that makes sure that the timer callback (all timer callbacks, to be precise; this is an implementation detail and could be adjusted later) is done and it is safe to destroy the object pointed by opaque. Once this function is available, the example above will look like this: timer_del(&timer); timer_join(&timer); free(opaque); Signed-off-by: Roman Kiryanov <r...@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. v4: add timer_join implmented via aio_wait_bh_oneshot as suggested by pbonz...@redhat.com. include/qemu/timer.h | 11 +++++++++++ util/qemu-timer.c | 14 ++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/include/qemu/timer.h b/include/qemu/timer.h index 5ce83c7911..5f7d673830 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -635,6 +635,17 @@ void timer_deinit(QEMUTimer *ts); */ void timer_del(QEMUTimer *ts); +/** + * timer_join: + * @ts: the timer + * + * Wait for the timer callback to finish (if it is runniing). + * + * This function is thread-safe but the timer and its timer list must not be + * freed while this function is running. + */ +void timer_join(QEMUTimer *ts); + /** * timer_free: * @ts: the timer diff --git a/util/qemu-timer.c b/util/qemu-timer.c index 213114be68..22759be580 100644 --- a/util/qemu-timer.c +++ b/util/qemu-timer.c @@ -30,6 +30,8 @@ #include "sysemu/replay.h" #include "sysemu/cpus.h" +#include "block/aio-wait.h" + #ifdef CONFIG_POSIX #include <pthread.h> #endif @@ -438,6 +440,18 @@ void timer_del(QEMUTimer *ts) } } +static void timer_join_noop(void *unused) {} + +/* Make sure the timer callback is done */ +void timer_join(QEMUTimer *ts) +{ + /* A side effect of aio_wait_bh_oneshot is all + * timer callbacks are done once it returns. + */ + aio_wait_bh_oneshot(qemu_get_aio_context(), + &timer_join_noop, NULL); +} + /* modify the current timer so that it will be fired when current_time >= expire_time. The corresponding callback will be called. */ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time) -- 2.45.2.803.g4e1b14247a-goog