|
From: | Paolo Bonzini |
Subject: | Re: [PATCH 1/3] util/qemu-timer: Make timer_free() imply timer_del() |
Date: | Tue, 15 Dec 2020 12:53:14 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 |
On 15/12/20 12:44, Peter Maydell wrote:
+static inline void timer_free(QEMUTimer *ts) +{ + + if (ts->expire_time != -1) { + timer_del(ts); + } + g_free(ts); +}I was thinking about this again this morning, and I'm not sure this is thread-safe.
It may not be thread-safe in principle, but any code that calls timer_mod, and isn't itself protected by a lock against timer_free, will be racing against the g_free immediately after. That is, that code could run after g_free and have a use-after-free bug.
But yes, I agree it is also an unnecessary optimization. It's better done in timer_del_locked, and removed from timer_mod_anticipate_ns. Since you are at it, you may also want to push the call to timer_del_locked down from timer_mod_ns and timer_mod_anticipate_ns to their callee, timer_mod_ns_locked.
Thanks, Paolo
[Prev in Thread] | Current Thread | [Next in Thread] |