From 4015d561c39d29200bf1ac542844fd5f3ba99426 Mon Sep 17 00:00:00 2001 From: Po Lu Date: Sat, 27 Aug 2022 09:54:16 +0800 Subject: [PATCH] Fix crash when handling "swallowed" generic events * src/xmenu.c (x_menu_translate_generic_event, x_menu_show): Pass through more events, correctly. * src/xterm.c (handle_one_xevent): Don't abort if must_free_data and xi_event is NULL; this is an Xlib bug. --- src/xmenu.c | 84 ++++++++++++++++++++++++++++++----------------------- src/xterm.c | 27 +++++++++++++++-- 2 files changed, 71 insertions(+), 40 deletions(-) diff --git a/src/xmenu.c b/src/xmenu.c index 5b8a8f77a2d..1452b3c6d12 100644 --- a/src/xmenu.c +++ b/src/xmenu.c @@ -242,45 +242,52 @@ x_menu_translate_generic_event (XEvent *event) { eassert (!event->xcookie.data); - if (XGetEventData (dpyinfo->display, &event->xcookie)) + switch (event->xcookie.evtype) { - switch (event->xcookie.evtype) - { - case XI_ButtonPress: - case XI_ButtonRelease: - xev = (XIDeviceEvent *) event->xcookie.data; - copy.xbutton.type = (event->xcookie.evtype == XI_ButtonPress - ? ButtonPress : ButtonRelease); - copy.xbutton.serial = xev->serial; - copy.xbutton.send_event = xev->send_event; - copy.xbutton.display = dpyinfo->display; - copy.xbutton.window = xev->event; - copy.xbutton.root = xev->root; - copy.xbutton.subwindow = xev->child; - copy.xbutton.time = xev->time; - copy.xbutton.x = lrint (xev->event_x); - copy.xbutton.y = lrint (xev->event_y); - copy.xbutton.x_root = lrint (xev->root_x); - copy.xbutton.y_root = lrint (xev->root_y); - copy.xbutton.state = xi_convert_event_state (xev); - copy.xbutton.button = xev->detail; - copy.xbutton.same_screen = True; - - device = xi_device_from_id (dpyinfo, xev->deviceid); - - /* I don't know the repercussions of changing - device->grab on XI_ButtonPress events, so be safe and - only do what is necessary to prevent the grab from - being left invalid as XMenuActivate swallows - events. */ - if (device && xev->evtype == XI_ButtonRelease) - device->grab &= ~(1 << xev->detail); - - XPutBackEvent (dpyinfo->display, ©); - - break; - } + case XI_ButtonPress: + case XI_ButtonRelease: + + if (!XGetEventData (dpyinfo->display, &event->xcookie)) + break; + + xev = (XIDeviceEvent *) event->xcookie.data; + copy.xbutton.type = (event->xcookie.evtype == XI_ButtonPress + ? ButtonPress : ButtonRelease); + copy.xbutton.serial = xev->serial; + copy.xbutton.send_event = xev->send_event; + copy.xbutton.display = dpyinfo->display; + copy.xbutton.window = xev->event; + copy.xbutton.root = xev->root; + copy.xbutton.subwindow = xev->child; + copy.xbutton.time = xev->time; + copy.xbutton.x = lrint (xev->event_x); + copy.xbutton.y = lrint (xev->event_y); + copy.xbutton.x_root = lrint (xev->root_x); + copy.xbutton.y_root = lrint (xev->root_y); + copy.xbutton.state = xi_convert_event_state (xev); + copy.xbutton.button = xev->detail; + copy.xbutton.same_screen = True; + + device = xi_device_from_id (dpyinfo, xev->deviceid); + + /* I don't know the repercussions of changing + device->grab on XI_ButtonPress events, so be safe and + only do what is necessary to prevent the grab from + being left invalid as XMenuActivate swallows + events. */ + if (device && xev->evtype == XI_ButtonRelease) + device->grab &= ~(1 << xev->detail); + + XPutBackEvent (dpyinfo->display, ©); XFreeEventData (dpyinfo->display, &event->xcookie); + + break; + + case XI_HierarchyChanged: + case XI_DeviceChanged: + /* These events must always be handled. */ + x_dispatch_event (event, dpyinfo->display); + break; } } } @@ -2783,6 +2790,9 @@ x_menu_show (struct frame *f, int x, int y, int menuflags, DEFER_SELECTIONS; XMenuActivateSetWaitFunction (x_menu_wait_for_event, FRAME_X_DISPLAY (f)); + /* When the input extension is in use, the owner_events grab will + report extension events on frames, which the XMenu library does + not normally understand. */ #ifdef HAVE_XINPUT2 XMenuActivateSetTranslateFunction (x_menu_translate_generic_event); #endif diff --git a/src/xterm.c b/src/xterm.c index b91d3a95173..c716d07adaf 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -20626,11 +20626,14 @@ handle_one_xevent (struct x_display_info *dpyinfo, bool must_free_data = false; XIEvent *xi_event = (XIEvent *) event->xcookie.data; + /* Sometimes the event is already claimed by GTK, which will free its data in due course. */ - if (!xi_event && XGetEventData (dpyinfo->display, &event->xcookie)) + if (!xi_event) { - must_free_data = true; + if (XGetEventData (dpyinfo->display, &event->xcookie)) + must_free_data = true; + xi_event = (XIEvent *) event->xcookie.data; } @@ -20638,7 +20641,25 @@ handle_one_xevent (struct x_display_info *dpyinfo, if (!xi_event) { - eassert (!must_free_data); + /* It may turn out that the event data has already been + implicitly freed for various reasons up to and + including XMenuActivate pushing some other event onto + the foreign-event queue, or x_menu_wait_for_events + calling XNextEvent through a timer that tries to wait + for input. + + In that case, XGetEventData will return True, but + cookie->data will be NULL. Since handling such input + events is not really important, we can afford to + discard them. + + The way Xlib is currently implemented makes calling + XFreeEventData unnecessary in this case, but call it + anyway, since not doing so may lead to a memory leak in + the future. */ + + if (must_free_data) + XFreeEventData (dpyinfo->display, &event->xcookie); goto OTHER; } -- 2.39.2