Re: [Qemu-devel] [PATCH 1/2] vl.c: set NULL upon deleting handlers in qemu_set_fd_handler2()

[Corentin Chary]

On Tue, Jan 25, 2011 at 10:03 AM, Stefan Hajnoczi <address@hidden> wrote:
> On Tue, Jan 25, 2011 at 8:33 AM, Corentin Chary
> <address@hidden> wrote:
>> From: Yoshiaki Tamura <address@hidden>
>>
>> Currently qemu_set_fd_handler2() is only setting ioh->deleted upon
>> deleting.  This may cause a crash when a read handler calls
>> qemu_set_fd_handler2() to delete handlers, but a write handler is
>> still invoked from main_loop_wait().  Because main_loop_wait() checks
>> handlers before calling, setting NULL upon deleting will protect
>> handlers being called if already deleted.
>>
>> One example is the new threaded vnc server.  When an error occurs in
>> the context of a read handler, it'll releases resources and deletes
>> handlers.  However, because the write handler still exists, it'll be
>> called, and then crashes because of lack of resources.  This patch
>> fixes it.
>
> Does this case still happen with qemu.git/master?  In November I sent
> a patch to check for deleted handlers:
>
> commit 0290b57bdfec83ca78b6d119ea9847bb17943328
> Author: Stefan Hajnoczi <address@hidden>
> Date:   Wed Nov 3 14:29:44 2010 +0000
>
>    Delete IOHandlers after potentially running them
>
>    Since commit 4bed9837309e58d208183f81d8344996744292cf an .fd_read()
>    handler that deletes its IOHandler is exposed to .fd_write() being
>    called on the deleted IOHandler.
>
>    This patch fixes deletion so that .fd_read() and .fd_write() are never
>    called on an IOHandler that is marked for deletion.
>
>    Signed-off-by: Stefan Hajnoczi <address@hidden>
>    Signed-off-by: Anthony Liguori <address@hidden>
>
> So I don't think Yoshi's patch is necessary anymore?

Ho I didn't see that one.
It's probably not necessary, but it stills make sense to apply this
patch since there is
absolutly no reasons to keep the old value in fd_read and fd_write when
the user explicitly asked to set them to NULL.



-- 
Corentin Chary
http://xf.iksaif.net