Re-reading: >> So we are calling a IOHandlerRecord::fd_write handler that is NULL. >> Looking at qemu_set_fd_handler2, this may happen if that function is >> called for an existing io-handler entry with non-NULL write handler, >> passing a NULL write and a non-NULL read handler. And all this without >> the global mutex held.
When using the vnc thread, nothing in the vnc thread will never be called directly by an IO-handler. So I don't really how in would trigger this. And since there is a lock for accessing the output buffer (and the network actually), there should not be any race condition either. So the real issue, is that qemu_set_fd_handler2() is called outside of the main thread by those two vnc_write() and vnc_flush() calls, without any kind of locking. > In upstream qemu, the latter - if it exists (which is not the case in > non-io-thread mode). > In qemu-kvm, those locks are yet unused. Rather the code in qemu-kvm.c > implements the global lock. So there is currently no lock for that when io-thread is disabled :/. Spice also seems to project this kind of thing with qemu_mutex_lock_iothread(). Maybe qemu_mutex_lock_iothread() should also be defined when CONFIG_VNC_THREAD=y ? > But I'm also not sure about vnc_send_framebuffer_update. Someone has to > go through the threaded vnc code again, very thoroughly. It looks fragile. while vnc-thread is enabled vnc_send_framebuffer_update() will always call vnc_write() with csock = -1 in a temporary buffer. Check vnc_async_encoding_start() and vnc_async_encoding_end(), they provide a kind of "sandbox" that prevent the thread to write anything the main-thread will use. You can also see that as a "transaction": the thread compute the update in a temporary buffer, and only send it to the network (real vnc_write calls with csock correctly set) once it's successfully finished. The is only two functions calls that break this isolation are the two that I pointed out earlier. -- Corentin Chary http://xf.iksaif.net
