[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/3] util/qemu-timer: Make timer_free() imply timer_del()
From: |
Peter Maydell |
Subject: |
Re: [PATCH 1/3] util/qemu-timer: Make timer_free() imply timer_del() |
Date: |
Tue, 15 Dec 2020 11:56:40 +0000 |
On Tue, 15 Dec 2020 at 11:53, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> 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.
I was thinking about potential races between the thread doing
the timer_free() and the iothread trying to run timers. Or
can that not happen ?
> 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.
One thing at a time :-)
thanks
-- PMM