[libvirt] [PATCH v3 3/5] tools: console: add missing locks in callbacks

[Nikolay Shirokovskiy]

On 04.04.2019 17:48, Cole Robinson wrote:
> On 4/4/19 2:57 AM, Nikolay Shirokovskiy wrote:
>>
>>
>> On 03.04.2019 23:44, Cole Robinson wrote:
>>> On 3/18/19 5:08 AM, Nikolay Shirokovskiy wrote:
>>>> Stream/fd callbacks accessing console object are called from the
>>>> event loop thread and the console object is also accessed from
>>>> the main thread so we are better add locking to handlers.
>>>>
>>>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
>>>> ---
>>>>  tools/virsh-console.c | 35 +++++++++++++++++++++++++----------
>>>>  1 file changed, 25 insertions(+), 10 deletions(-)
>>>>
>>>
>>> The qemuAgentIO and qemuMonitorIO callbacks also add have matching
>>> Ref/Unref calls along side the lock calls, but after scratching my head
>>> over it for a while I don't think it's necessary here: even if
>>> virConsoleShutdown is called from one of the callbacks, the main thread
>>> will still hold a reference until the callback releases the object lock
>>>
>>
>> We don't need even to rely on main thread. Event loop itself has a reference
>> which will be unrefed only after callback returns.
>>
> 
> Hmm interesting, where in the code is that exactly? I know we pass a
> Free callback to virEventAddHandle but that's only called when the
> handle is removed, not after the callback is invoked. Maybe I'm missing
> something
> 

The reason we are ok is that when we call virEventPollRemoveHandle it only
marks handle to be removed so that if this call is done from callback we 
don't free opaque object immediately. Only after callback is finished
we call virEventPollCleanupHandles which actually do freeing.

Nikolay