On Thu, 22 Nov 2018 at 07:32, Gerd Hoffmann <kra...@redhat.com> wrote: > > Hi, > > > Something somewhere in this call chain should have taken > > the iothread lock, I assume, but I'm not sure where. > > > > Gerd -- what are the rules of the UI subsystem regarding > > multiple threads, and what threads are allowed to call > > UI functions like qemu_input_event_send_key_qcode(), > > from which threads, and whether they need to eg hold > > the iothread lock when they do so? There's no > > documentation on these functions in include/ui/input.h. > > UI event handling code typically runs in iothread context. So, yes, > when calling qemu_input_* the UI code holds the iothread lock. > > > (To fix a problem on OSX Mojave this patchset has moved > > which thread the UI executes on, so it is now always > > the main thread and not the thread which calls > > the QemuDisplay 'init' callback. That seems to break > > an implicit assumption in the UI layer.) > > Hmm, I guess the options are (a) grab the iothread lock before calling > input layer functions, or (b) queue the event and schedule a bottom half > which processes the queue (which will then be called from iothread > context, with the lock already taken).
I was thinking about this today, and I realized that if we just make the OSX UI thread code grab the iothread lock, then there is a potential deadlock with the changes (also in this patchset) which defer the DisplayChangeListener update/switch/refresh operations to the UI thread. The current patchset has those be synchronous deferrals, so you could get a deadlock if the UI thread is waiting for the iothread lock, but the code in the UI midlayer is holding the iothread lock and waiting for a DCL operation to be run on the main thread. What are the required semantics for update/switch/refresh? These don't seem to be documented. Can we validly make those be asychronous, or does the midlayer require that the operation has completed and been reflected onscreen before the update/switch/refresh callback returns ? If those have to be synchronous, then we'll need to do something more complicated (eg the queuing of events that you suggest), which I'd prefer it if we can avoid, because that implies memory allocation or a fixed length queue (plus some fairly significant restructuring of the cocoa frontend). thanks -- PMM
