[PATCH 1/1] mieq: Fix a crash regression in mieqProcessDeviceEvent

[Jeremy Huddleston Sequoia]

On Jul 21, 2014, at 19:09, Peter Hutterer <peter.hutterer at who-t.net> wrote:

> On Sat, Jul 19, 2014 at 05:36:34PM -0700, Jeremy Huddleston Sequoia wrote:
>> Actually ... while this does address the crash, it now just causes a bunch
>> of our events to be ignored instead of processed.
>> 
>> So should that instead be a 'dev && !dev->enabled' or do we need to
>> instead make a fake device for our generic darwin events like you did in
>> test/input.c?
> 
>> 
>> If the latter, ProcessInputEvents gives the implication that dev can be
>> NULL whereas mieqProcessInputEvents does not handle that, so we should
>> possibly assert that dev is non-null earlier and remove dead code for the
>> NULL case.
> 
> I've got a comment here in CopyGetMasterEvent hat ET_XQuartz has a null
> device, so we did handle NULL events before. Which means that the condition
> you said above should be the best fix, the rest of the code seems to be fine
> with handling NULL devices.
> 
> Requiring non-NULL device means you have to set a device for things like
> ControllerNotify or PasteboardNotify, which isn't the right solution either.
> 
> What I'm wondering though: the reason the EQ exists is because we expect the
> queueing side to be inside a signal handler. I take it you're using the EQ
> here for inter-thread communication, you can't process the events directly?

No, we can't process the events directly on other threads.  In these cases, we need the "server thread" to do something (RandR changes, rootless changes, termination, etc) because the server needs to serialize access to various data structures on the server thread.


> 
> Cheers,
>   Peter
> 
> 
>> On Jul 19, 2014, at 17:12, Jeremy Huddleston Sequoia <jeremyhu at apple.com> wrote:
>> 
>>> (lldb) bt
>>> * thread #6: tid = 0x92d4eb, 0x00000001001dee94 X11.bin`mieqProcessDeviceEvent(dev=0x0000000000000000, event=0x0000000100298bb0,
>>> screen=0x0000000000000000) + 36 at mieq.c:519, stop reason = EXC_BAD_ACCESS (code=1, address=0x44)
>>> * frame #0: 0x00000001001dee94 X11.bin`mieqProcessDeviceEvent(dev=0x0000000000000000, event=0x0000000100298bb0, screen=0x0000000000000000) + 36 at
>>> mieq.c:519
>>>   frame #1: 0x00000001001df3eb X11.bin`mieqProcessInputEvents + 555 at mieq.c:631
>>>   frame #2: 0x0000000100017674 X11.bin`ProcessInputEvents + 20 at darwinEvents.c:422
>>>   frame #3: 0x0000000100175eaa X11.bin`Dispatch + 154 at dispatch.c:357
>>>   frame #4: 0x0000000100181b4a X11.bin`dix_main(argc=4, argv=0x00007fff5fbff750, envp=0x00007fff5fbff650) + 1594 at main.c:296
>>>   frame #5: 0x000000010001ba80 X11.bin`server_thread(arg=0x0000000101208220) + 64 at quartzStartup.c:66
>>>   frame #6: 0x00007fff89bb9899 libsystem_pthread.dylib`_pthread_body + 138
>>>   frame #7: 0x00007fff89bb972a libsystem_pthread.dylib`_pthread_start + 137
>>>   frame #8: 0x00007fff89bbdfc9 libsystem_pthread.dylib`thread_start + 13
>>> 
>>> Regression from: 9fb08310b51b46736f3ca8dbc04efdf502420403
>>> 
>>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu at apple.com>
>>> CC: Peter Hutterer <peter.hutterer at who-t.net>
>>> ---
>>> mi/mieq.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/mi/mieq.c b/mi/mieq.c
>>> index 872ff93..aced60d 100644
>>> --- a/mi/mieq.c
>>> +++ b/mi/mieq.c
>>> @@ -516,7 +516,7 @@ mieqProcessDeviceEvent(DeviceIntPtr dev, InternalEvent *event, ScreenPtr screen)
>>>    verify_internal_event(event);
>>> 
>>>    /* refuse events from disabled devices */
>>> -    if (!dev->enabled)
>>> +    if (!dev || !dev->enabled)
>>>        return;
>>> 
>>>    /* Custom event handler */
>>> -- 
>>> 2.0.1
>>> 
>>