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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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)
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]