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]

 



So the above patches are pushed now. What about the last one? I've elaborated
on its meaning in sibling letter.

Nikolay

On 28.10.2016 00:58, John Ferlan wrote:
> 
> 
> 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]