[PATCH 2/4] X event queue mutex
Jeremy Huddleston
jeremyhu at freedesktop.org
Thu Oct 23 14:08:18 PDT 2008
Whoops... I forgot the diff to show you what I meant... here's what
I'm going to push to 1.4-apple to address our problem.
--Jeremy
diff --git a/mi/mieq.c b/mi/mieq.c
index 3f50f27..543b7e7 100644
--- a/mi/mieq.c
+++ b/mi/mieq.c
@@ -231,7 +231,7 @@ mieqSetHandler(int event, mieqHandler handler)
void
mieqProcessInputEvents(void)
{
- EventRec *e = NULL;
+ EventRec e;
int x = 0, y = 0;
DeviceIntPtr dev = NULL;
@@ -250,45 +250,46 @@ mieqProcessInputEvents(void)
DPMSSet(DPMSModeOn);
#endif
- e = &miEventQueue.events[miEventQueue.head];
+ memcpy(&e, &miEventQueue.events[miEventQueue.head],
sizeof(EventRec));
miEventQueue.head = (miEventQueue.head + 1) % QUEUE_SIZE;
- if (miEventQueue.handlers[e->event->u.u.type]) {
+#ifdef XQUARTZ
+ pthread_mutex_unlock(&miEventQueueMutex);
+#endif
+
+ if (miEventQueue.handlers[e.event->u.u.type]) {
/* If someone's registered a custom event handler, let
them
* steal it. */
- miEventQueue.handlers[e->event->u.u.type]
(miEventQueue.pDequeueScreen->myNum,
- e->event, dev,
- e->nevents);
+ miEventQueue.handlers[e.event->u.u.type](e.pScreen->myNum,
+ e.event, e.pDev,
+ e.nevents);
}
- else if (e->pScreen != miEventQueue.pDequeueScreen) {
+ else if (e.pScreen != miEventQueue.pDequeueScreen) {
/* Assumption - screen switching can only occur on
motion events. */
- miEventQueue.pDequeueScreen = e->pScreen;
- x = e->event[0].u.keyButtonPointer.rootX;
- y = e->event[0].u.keyButtonPointer.rootY;
- NewCurrentScreen (miEventQueue.pDequeueScreen, x, y);
+ miEventQueue.pDequeueScreen = e.pScreen;
+ x = e.event[0].u.keyButtonPointer.rootX;
+ y = e.event[0].u.keyButtonPointer.rootY;
+ NewCurrentScreen(e.pScreen, x, y);
}
else {
/* If this is a core event, make sure our keymap, et al,
is
* changed to suit. */
- if (e->event[0].u.u.type == KeyPress ||
- e->event[0].u.u.type == KeyRelease) {
- SwitchCoreKeyboard(e->pDev);
+ if (e.event[0].u.u.type == KeyPress ||
+ e.event[0].u.u.type == KeyRelease) {
+ SwitchCoreKeyboard(e.pDev);
dev = inputInfo.keyboard;
}
- else if (e->event[0].u.u.type == MotionNotify ||
- e->event[0].u.u.type == ButtonPress ||
- e->event[0].u.u.type == ButtonRelease) {
- SwitchCorePointer(e->pDev);
+ else if (e.event[0].u.u.type == MotionNotify ||
+ e.event[0].u.u.type == ButtonPress ||
+ e.event[0].u.u.type == ButtonRelease) {
+ SwitchCorePointer(e.pDev);
dev = inputInfo.pointer;
}
else {
- dev = e->pDev;
+ dev = e.pDev;
}
- dev->public.processInputProc(e->event, dev, e->nevents);
+ dev->public.processInputProc(e.event, dev, e.nevents);
}
}
-#ifdef XQUARTZ
- pthread_mutex_unlock(&miEventQueueMutex);
-#endif
}
On Oct 23, 2008, at 13:59, Jeremy Huddleston wrote:
> Hey Tiago,
>
> I hope things are going well for you. I've recently hit an issue
> using locks in miEq. We're doing it the same way in mieq.c as your
> proposal (patch 2/4) and this causes us to hit a deadlock when the
> enqueueing thread is waiting for the lock to push an event and the
> dequeuing thread (the server thread) is processing an event that
> requires a message to be sent to the enqueueing thread.
>
> I am going to try solving this by making the lock a bit more
> conservative in mieqProcessInputEvents. Our current implementation
> (we're still on 1.4) passes the event to the handler as a pointer to
> the event within the queue. In 1.5, mieqProcessInputEvents now
> copies the nevents first... so we might be able to release that a
> bit sooner...
>
> The thing is that we have:
>
> e = &miEventQueue.events[miEventQueue.head];
>
> Then e->events is copied, but 'e' itself is still directly
> referenced throughout mieqProcessInputEvents. Could we actually
> just copy all of miEventQueue.events[miEventQueue.head] to a local
> copy and release the lock after that? I see us using:
>
> e->pDev
> e->nevents
> e->events (already copied in 1.5.x and later if nevents > 1)
>
> --Jeremy
>
> Begin forwarded message:
>
>> From: Jeremy Huddleston <jeremyhu at apple.com>
>> Date: October 19, 2008 12:59:16 PDT
>> To: Kevin Van Vechten <kvv at apple.com>, Jordan Hubbard <jkh at apple.com>
>> Cc: George Peter Staplin <gstaplin at apple.com>
>> Subject: mach_msg async? thread communication question
>>
>> So... I've made some progress with fullscreen...
>>
>> It now renders the "weaved" background and only crashes if there
>> are windows other than the root window... but if you just want to
>> stare at a black and white X11 weave background, we're there!
>>
>> Aside from this crash, I noticed a thread communication deadlock
>> issue (see the stack traces below):
>>
>> Thread 2's mieqProcessInputEvents and mieqEnqueue lock the input
>> event queue... but some of the handlers for mieqProcessInputEvents
>> actually need to msg the Appkit thread which appears to cause the
>> deadlock.
>>
>> Is there a way to allow mach_msg to be async? In these cases, we
>> don't care about return values.
>>
>> Or do I need to do something clever (read: annoying) to address
>> this issue?
>>
>> Call graph:
>> 4783 Thread_2503
>> 4783 start
>> 4783 main
>> 4783 mach_msg_server
>> 4783 mach_startup_server
>> 4783 _Xstart_x11_server
>> 4783 do_start_x11_server
>> 4783 server_main
>> 4783 X11ControllerMain
>> 4783 X11ApplicationMain
>> 4783 -[NSApplication run]
>> 4783 -[X11Application sendEvent:]
>> 4783 -[NSApplication sendEvent:]
>> 4783 -[NSApplication
>> _handleKeyEquivalent:]
>> 4783 -[NSMenu performKeyEquivalent:]
>> 4783 -[NSCarbonMenuImpl
>> performActionWithHighlightingForItemAtIndex:]
>> 4783 -[NSMenu
>> performActionForItemAtIndex:]
>> 4783 -[NSApplication
>> sendAction:to:from:]
>> 4783 -[X11Controller
>> toggle_fullscreen:]
>> 4783 DarwinSendDDXEvent
>> 4783 mieqEnqueue
>> 4768 pthread_mutex_lock
>> 4768
>> semaphore_wait_trap
>> 4768
>> semaphore_wait_trap
>> 15 0xffffffff
>> 15 _sigtramp
>> 15 _sigtramp
>> 4783 Thread_2703
>> 4783 thread_start
>> 4783 _pthread_start
>> 4783 server_thread
>> 4783 dix_main
>> 4783 Dispatch
>> 4783 ProcessInputEvents
>> 4783 mieqProcessInputEvents
>> 4783 DarwinEventHandler
>> 4783 QuartzHide
>> 4783 QuartzSetFullscreen
>> 4783 X11ApplicationShowHideMenubar
>> 4783 message_kit_thread
>> 4783 mach_msg
>> 4783 mach_msg_trap
>> 4783 mach_msg_trap
>>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3040 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20081023/fb9bd22d/attachment.bin>
More information about the xorg
mailing list