reentrancy of mieqProcessInputEvents()?

Andy Ritger aritger at nvidia.com
Tue Jun 12 17:11:49 PDT 2012


Is mi/mieq.c:mieqProcessInputEvents() expected to be reentrant?  Or should
mieqProcessInputEvents() never be called in that situation?

More specifically, is there a reason that the 'event' local variable in
mieqProcessInputEvents() is declared static?

    static InternalEvent event;

Consider the following sequence:

(a) User hits ctrl-alt-+ to do an old-school mode switch, and the keyboard
    event is put on the event queue.

(b) mieqProcessInputEvents() pops the keyboard event from the event queue,
    storing the event data in a static local variable, and then passing a
    pointer to that variable into mieqProcessDeviceEvent():

    mieqProcessInputEvents(void)
    {
        [...]
        static InternalEvent event;
        [...]
        while (miEventQueue.head != miEventQueue.tail) {
            e = &miEventQueue.events[miEventQueue.head];

            event = *e->events;
            [...]
            mieqProcessDeviceEvent(dev, &event, screen);
        [...]
    }

(c) The event pointer gets passed through several functions until we
    hit XkbHandleActions():

#34 0x0000000000585596 in XkbHandleActions (dev=0x1176370, kbd=0x1176370, event=0x85cf00) at xkbActions.c:1234
#35 0x00000000005865d8 in XkbProcessKeyboardEvent (event=0x85cf00, keybd=0x1176370) at xkbPrKeyEv.c:146
#36 0x000000000057ad28 in AccessXFilterPressEvent (event=0x85cf00, keybd=0x1176370) at xkbAccessX.c:569
#37 0x000000000058678e in ProcessKeyboardEvent (ev=0x85cf00, keybd=0x1176370) at xkbPrKeyEv.c:181
#38 0x00000000005aec39 in mieqProcessDeviceEvent (dev=0x1176370, event=0x85cf00, screen=0xaa49e0) at mieq.c:557
#39 0x00000000005aee75 in mieqProcessInputEvents () at mieq.c:624
#40 0x000000000048adb9 in ProcessInputEvents () at xf86Events.c:164
#41 0x0000000000430835 in Dispatch () at dispatch.c:353
#42 0x0000000000421a45 in main (argc=1, argv=0x7fff049880d8, envp=0x7fff049880e8) at main.c:288

(d) XkbHandleActions() will look at the event type to decide it is a
    keyEvent type, query the key action type, perform some operation based
    on the key action type, then pass the event into FixKeyState():

    void
    XkbHandleActions(DeviceIntPtr dev, DeviceIntPtr kbd, DeviceEvent *event)
    {
        [...]
        keyEvent = ((event->type == ET_KeyPress) || (event->type == ET_KeyRelease));
        [...]
        act = XkbGetKeyAction(xkbi, &xkbi->state, key);
        [...]
        switch (act.type) {
            [...]
            case XkbSA_XFree86Private:
                filter = _XkbNextFreeFilter(xkbi);
                sendEvent = _XkbFilterXF86Private(xkbi, filter, key, &act);
                break;
            }
        }
        [...]
        if (keyEvent) {
            FixKeyState(event, dev);
        }
        [...]
    }

(e) FixKeyState() does this:

    if (event->type == ET_KeyPress)
        set_key_down(keybd, key, KEY_PROCESSED);
    else if (event->type == ET_KeyRelease)
        set_key_up(keybd, key, KEY_PROCESSED);
    else
        FatalError("Impossible keyboard event");

(that will be important later...)

(f) Back in XkbHandleActions(), we called _XkbFilterXF86Private which
    passes the key event to the XFree86 layer, which decodes the key event as
    "+vmode", and we call through the pScrnInfo->SwitchMode wrap chain:

#6  0x00000000004df525 in xf86CursorSwitchMode (index=0, mode=0x1c54eb0, flags=0) at xf86Cursor.c:253
#7  0x0000000000496ef1 in CMapSwitchMode (index=0, mode=0x1c54eb0, flags=0) at xf86cmap.c:492
#8  0x0000000000485fb1 in xf86SwitchMode (pScreen=0x1c40c00, mode=0x1c54eb0) at xf86Cursor.c:232
#9  0x00000000004863f8 in xf86ZoomViewport (pScreen=0x1c40c00, zoom=1) at xf86Cursor.c:332
#10 0x000000000048ae8e in xf86ProcessActionEvent (action=ACTION_NEXT_MODE, arg=0x0) at xf86Events.c:191
#11 0x00000000004eb36c in XkbDDXPrivate (dev=0x22ac580, key=86 'V', act=0x7ffff8e60770) at xkbPrivate.c:32
#12 0x0000000000584746 in _XkbFilterXF86Private (xkbi=0x22ad470, filter=0x2265400, keycode=86,
    pAction=0x7ffff8e60770) at xkbActions.c:959
#13 0x0000000000585596 in XkbHandleActions (dev=0x22ac580, kbd=0x22ac580, event=0x85cf00)
    at xkbActions.c:1234

(g) Someone in the pScrnInfo->SwitchMode wrap chain calls RRTellChanged()
    (in my case, it is the NVIDIA X driver, but I think many
    drivers could be exposed to this through xf86SetSingleMode() ->
    xf86RandR12TellChanged()) which calls UpdateCurrentTime().

(h) UpdateCurrentTime() conditionally calls ProcessInputEvents():

    void
    UpdateCurrentTime(void)
    {
        [...]
        if (*checkForInput[0] != *checkForInput[1])
            ProcessInputEvents();
        [...]
    }

    ---

    void
    ProcessInputEvents(void)
    {
        int x, y;

        mieqProcessInputEvents();

        /* FIXME: This is a problem if we have multiple pointers */
        miPointerGetPosition(inputInfo.pointer, &x, &y);

        xf86SetViewport(xf86Info.currentScreen, x, y);
    }

(i) checkForInput[0] and [1] point to miEventQueue.{head,tail}; and tail
    potentially got updated earlier in this sequence:

0x00000000005ae0aa in mieqEnqueue (pDev=0x2309c00, e=0x26e2e20) at mieq.c:322
322         miEventQueue.tail = (oldtail + 1) % miEventQueue.nevents;
#0  0x00000000005ae0aa in mieqEnqueue (pDev=0x2309c00, e=0x26e2e20) at mieq.c:322
#1  0x00000000005baae7 in miPointerMove (pDev=0x2309c00, pScreen=0x21dc7f0, x=1600, y=600) at mipointer.c:703
#2  0x00000000005b9f0c in miPointerWarpCursor (pDev=0x2309c00, pScreen=0x21dc7f0, x=1600, y=600) at mipointer.c:371
#3  0x00000000004875e1 in xf86WarpCursor (pDev=0x2309c00, pScreen=0x21dc7f0, x=1600, y=600) at xf86Cursor.c:476
#4  0x0000000000486f65 in xf86SwitchMode (pScreen=0x21dc7f0, mode=0x21fac80) at xf86Cursor.c:283
#5  0x000000000048712b in xf86ZoomViewport (pScreen=0x21dc7f0, zoom=1) at xf86Cursor.c:333
#6  0x000000000048bba2 in xf86ProcessActionEvent (action=ACTION_NEXT_MODE, arg=0x0) at xf86Events.c:191
#7  0x00000000004ebed8 in XkbDDXPrivate (dev=0x26bbc00, key=86 'V', act=0x7fff8e43bc70) at xkbPrivate.c:32
#8  0x00000000005845ae in _XkbFilterXF86Private (xkbi=0x26bcaf0, filter=0x233b650, keycode=86, pAction=0x7fff8e43bc70)
    at xkbActions.c:958
#9  0x0000000000585119 in XkbHandleActions (dev=0x26bbc00, kbd=0x26bbc00, event=0x85c500) at xkbActions.c:1194

    So, UpdateCurrentTime() may call ProcessInputEvents(), which will reenter
    mieqProcessInputEvents().

(j) mieqProcessInputEvents() will overwrite the contents of the static
    'event' variable, such that when we finally get back up to
    XkbHandleActions() and it calls FixKeyState(), the contents of
    'event' are no longer consistent with the value of keyEvent and we
    hit the FatalError() case.

Making the 'event' variable in mieqProcessInputEvents() non-static
avoids the FatalError() in FixKeyState(), but is it problematic to
have the re-entered instance of mieqProcessInputEvents() process events
from miEventQueue before the first instance of mieqProcessInputEvents()
has completed processing of its current event?

Thanks,
- Andy



More information about the xorg-devel mailing list