Re: [libvirt] [PATCH v2 3/3] util: event loop: document another reason to defer deletion

On 10/04/2016 10:27 AM, Nikolay Shirokovskiy wrote: > This is why we should not free callback object synchronously > upon removing handle or timeout. Imagine: > > 1. loop thread, drops the lock and is about to call event callback > 2. another thread, enters remove handle and frees callback object > 3. loop thread, enters event callback, uses callback object BOOOM > --- > src/util/vireventpoll.c | 7 +++++++ > 1 file changed, 7 insertions(+) > I'm having difficulty trying to decipher the point you're trying to make in the context of the existing comments, the previous upstream series, and the cover letter explanation. While not explicitly stated for each, the 'flag' that's set is 'deleted'. Once the 'deleted' flag is set, it's expected that the object will be reaped later when it's safer to do so (such as virEventPollCleanupTimeouts and virEventPollCleanupHandles) via the 'ff' passed during virEventAddHandle and virEventAddTimeout. I'm going to pass on pushing this one. John > diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c > index 81ecab4..802b679 100644 > --- a/src/util/vireventpoll.c > +++ b/src/util/vireventpoll.c > @@ -176,6 +176,10 @@ void virEventPollUpdateHandle(int watch, int events) > * Unregister a callback from a file handle > * NB, it *must* be safe to call this from within a callback > * For this reason we only ever set a flag in the existing list. > + * Another reason is that as we drop the lock upon event callback > + * invocation we can't free callback object if we called > + * from a thread then loop thread. > + * > * Actual deletion will be done out-of-band > */ > int virEventPollRemoveHandle(int watch) > @@ -295,6 +299,9 @@ void virEventPollUpdateTimeout(int timer, int frequency) > * Unregister a callback for a timer > * NB, it *must* be safe to call this from within a callback > * For this reason we only ever set a flag in the existing list. > + * Another reason is that as we drop the lock upon event callback > + * invocation we can't free callback object if we called > + * from a thread then loop thread. > * Actual deletion will be done out-of-band > */ > int virEventPollRemoveTimeout(int timer) >