From 6f940c6a1183dd1660f35e6c82d33183a6892cb4 Mon Sep 17 00:00:00 2001 From: Po Lu Date: Mon, 22 May 2023 11:52:33 +0800 Subject: [PATCH] Plug various leaks and fix input method initialization * src/image.c (free_bitmap_record): Free bm->name correctly even if the display connection has been closed. * src/xfns.c (x_window): Use dpyinfo-specific `use_xim' flag. * src/xterm.c (use_xim): Remove global variable. (xim_destroy_callback): Free `xim_styles' if present, and set it to NULL to be on the safe side. (xim_open_dpy): Consult dpyinfo->use_xim instead. Don't leak `xim_styles' if an IM was previously opened. (xim_initialize): Use dpyinfo-specific `use_xim' flag. (xim_close_dpy): Check if `dpyinfo->xim_callback_data' is set before unregistering the instantiation callback. (x_term_init): Determine whether or not to use XIM for each display opened, instead of using the resources of the last display opened to toggle a global flag. (x_delete_terminal): Always call `image_destroy_all_bitmaps' and `xim_close_dpy'. Free storage used to hold bitmap records. * src/xterm.h (struct x_display_info): New field `use_xim'. --- src/image.c | 14 ++++- src/xfns.c | 56 +++++++++---------- src/xterm.c | 156 ++++++++++++++++++++++++++++++++++------------------ src/xterm.h | 11 ++-- 4 files changed, 148 insertions(+), 89 deletions(-) diff --git a/src/image.c b/src/image.c index 87a0c1ca497..c9420b48f4a 100644 --- a/src/image.c +++ b/src/image.c @@ -842,9 +842,17 @@ static void free_bitmap_record (Display_Info *dpyinfo, Bitmap_Record *bm) { #ifdef HAVE_X_WINDOWS - XFreePixmap (dpyinfo->display, bm->pixmap); - if (bm->have_mask) - XFreePixmap (dpyinfo->display, bm->mask); + /* Free the pixmap and mask. Only do this if DPYINFO->display is + still set, which may not be the case if the connection has + already been closed in response to an IO error. */ + + if (dpyinfo->display) + { + XFreePixmap (dpyinfo->display, bm->pixmap); + if (bm->have_mask) + XFreePixmap (dpyinfo->display, bm->mask); + } + #ifdef USE_CAIRO if (bm->stipple) cairo_pattern_destroy (bm->stipple); diff --git a/src/xfns.c b/src/xfns.c index 9e004f6a678..234a48c908f 100644 --- a/src/xfns.c +++ b/src/xfns.c @@ -4252,9 +4252,9 @@ x_window (struct frame *f, long window_prompting) #ifdef HAVE_X_I18N FRAME_XIC (f) = NULL; - if (use_xim) + if (FRAME_DISPLAY_INFO (f)->use_xim) create_frame_xic (f); -#endif +#endif /* HAVE_X_I18N */ f->output_data.x->wm_hints.input = True; f->output_data.x->wm_hints.flags |= InputHint; @@ -4355,32 +4355,32 @@ x_window (struct frame *f) #ifdef HAVE_X_I18N FRAME_XIC (f) = NULL; - if (use_xim) - { - block_input (); - create_frame_xic (f); - if (FRAME_XIC (f)) - { - /* XIM server might require some X events. */ - unsigned long fevent = NoEventMask; - XGetICValues (FRAME_XIC (f), XNFilterEvents, &fevent, NULL); + if (FRAME_DISPLAY_INFO (f)->use_xim) + { + block_input (); + create_frame_xic (f); + if (FRAME_XIC (f)) + { + /* XIM server might require some X events. */ + unsigned long fevent = NoEventMask; + XGetICValues (FRAME_XIC (f), XNFilterEvents, &fevent, NULL); - if (fevent != NoEventMask) - { - XSetWindowAttributes attributes; - XWindowAttributes wattr; - unsigned long attribute_mask; - - XGetWindowAttributes (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), - &wattr); - attributes.event_mask = wattr.your_event_mask | fevent; - attribute_mask = CWEventMask; - XChangeWindowAttributes (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), - attribute_mask, &attributes); - } - } - unblock_input (); - } + if (fevent != NoEventMask) + { + XSetWindowAttributes attributes; + XWindowAttributes wattr; + unsigned long attribute_mask; + + XGetWindowAttributes (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), + &wattr); + attributes.event_mask = wattr.your_event_mask | fevent; + attribute_mask = CWEventMask; + XChangeWindowAttributes (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f), + attribute_mask, &attributes); + } + } + unblock_input (); + } #endif append_wm_protocols (FRAME_DISPLAY_INFO (f), f); @@ -4427,7 +4427,7 @@ x_window (struct frame *f) initial_set_up_x_back_buffer (f); #ifdef HAVE_X_I18N - if (use_xim) + if (FRAME_DISPLAY_INFO (f)->use_xim) { create_frame_xic (f); if (FRAME_XIC (f)) diff --git a/src/xterm.c b/src/xterm.c index 15bd9f98d17..0450230efe6 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -798,13 +798,6 @@ typedef int (*Emacs_XIOErrorHandler) (Display *); #define USE_CAIRO_XCB_SURFACE #endif -/* Default to using XIM if available. */ -#ifdef USE_XIM -bool use_xim = true; -#else -bool use_xim = false; /* configure --without-xim */ -#endif - #if XCB_SHAPE_MAJOR_VERSION > 1 \ || (XCB_SHAPE_MAJOR_VERSION == 1 && \ XCB_SHAPE_MINOR_VERSION >= 1) @@ -26666,7 +26659,12 @@ xim_destroy_callback (XIM xim, XPointer client_data, XPointer call_data) /* No need to call XCloseIM. */ dpyinfo->xim = NULL; - XFree (dpyinfo->xim_styles); + + /* Also free IM values; those are allocated separately upon + XGetIMValues. */ + if (dpyinfo->xim_styles) + XFree (dpyinfo->xim_styles); + dpyinfo->xim_styles = NULL; unblock_input (); } @@ -26684,10 +26682,20 @@ xim_open_dpy (struct x_display_info *dpyinfo, char *resource_name) XIM xim; const char *locale; - if (use_xim) + if (dpyinfo->use_xim) { if (dpyinfo->xim) - XCloseIM (dpyinfo->xim); + { + XCloseIM (dpyinfo->xim); + + /* Free values left over from the last time the IM + connection was established. */ + + if (dpyinfo->xim_styles) + XFree (dpyinfo->xim_styles); + dpyinfo->xim_styles = NULL; + } + xim = XOpenIM (dpyinfo->display, dpyinfo->rdb, resource_name, emacs_class); dpyinfo->xim = xim; @@ -26716,7 +26724,6 @@ xim_open_dpy (struct x_display_info *dpyinfo, char *resource_name) build_string (locale)); } } - else #endif /* HAVE_XIM */ dpyinfo->xim = NULL; @@ -26785,7 +26792,7 @@ xim_initialize (struct x_display_info *dpyinfo, char *resource_name) { dpyinfo->xim = NULL; #ifdef HAVE_XIM - if (use_xim) + if (dpyinfo->use_xim) { #ifdef HAVE_X11R6_XIM struct xim_inst_t *xim_inst = xmalloc (sizeof *xim_inst); @@ -26794,15 +26801,19 @@ xim_initialize (struct x_display_info *dpyinfo, char *resource_name) dpyinfo->xim_callback_data = xim_inst; xim_inst->dpyinfo = dpyinfo; xim_inst->resource_name = xstrdup (resource_name); - ret = XRegisterIMInstantiateCallback - (dpyinfo->display, dpyinfo->rdb, xim_inst->resource_name, - emacs_class, xim_instantiate_callback, - /* This is XPointer in XFree86 but (XPointer *) on Tru64, at - least, but the configure test doesn't work because - xim_instantiate_callback can either be XIMProc or - XIDProc, so just cast to void *. */ - (void *) xim_inst); - eassert (ret == True); + + /* The last argument is XPointer in XFree86 but (XPointer *) on + Tru64, at least, but the configure test doesn't work because + xim_instantiate_callback can either be XIMProc or XIDProc, so + just cast to void *. */ + + ret = XRegisterIMInstantiateCallback (dpyinfo->display, + dpyinfo->rdb, + xim_inst->resource_name, + emacs_class, + xim_instantiate_callback, + (void *) xim_inst); + eassert (ret); #else /* not HAVE_X11R6_XIM */ xim_open_dpy (dpyinfo, resource_name); #endif /* not HAVE_X11R6_XIM */ @@ -26811,32 +26822,56 @@ xim_initialize (struct x_display_info *dpyinfo, char *resource_name) } -/* Close the connection to the XIM server on display DPYINFO. */ +/* Close the connection to the XIM server on display DPYINFO. + Unregister any IM instantiation callback previously installed, + close the connection to the IM server if possible, and free any + retrieved IM values. */ static void xim_close_dpy (struct x_display_info *dpyinfo) { #ifdef HAVE_XIM - if (use_xim) - { #ifdef HAVE_X11R6_XIM - struct xim_inst_t *xim_inst = dpyinfo->xim_callback_data; + struct xim_inst_t *xim_inst; + Bool rc; + + /* If dpyinfo->xim_callback_data is not set, then IM support wasn't + initialized, which can happen if Xlib doesn't understand the C + locale being used. */ + + if (dpyinfo->xim_callback_data) + { + xim_inst = dpyinfo->xim_callback_data; if (dpyinfo->display) { - Bool ret = XUnregisterIMInstantiateCallback - (dpyinfo->display, dpyinfo->rdb, xim_inst->resource_name, - emacs_class, xim_instantiate_callback, (void *) xim_inst); - eassert (ret == True); + rc = XUnregisterIMInstantiateCallback (dpyinfo->display, + dpyinfo->rdb, + xim_inst->resource_name, + emacs_class, + xim_instantiate_callback, + (void *) xim_inst); + eassert (rc); } + xfree (xim_inst->resource_name); xfree (xim_inst); -#endif /* HAVE_X11R6_XIM */ - if (dpyinfo->display) - XCloseIM (dpyinfo->xim); - dpyinfo->xim = NULL; - XFree (dpyinfo->xim_styles); } +#endif /* HAVE_X11R6_XIM */ + + /* Now close the connection to the input method server. This may + access the display connection, and isn't safe if the display has + already been closed. */ + + if (dpyinfo->display && dpyinfo->xim) + XCloseIM (dpyinfo->xim); + dpyinfo->xim = NULL; + + /* Free the list of XIM styles retrieved. */ + + if (dpyinfo->xim_styles) + XFree (dpyinfo->xim_styles); + dpyinfo->xim_styles = NULL; #endif /* HAVE_XIM */ } @@ -30766,14 +30801,6 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name) dpyinfo->fixes_pointer_blanking = (egetenv ("EMACS_XFIXES") != NULL); #endif -#ifdef HAVE_X_I18N - /* Avoid initializing input methods if the X library does not - support Emacs's locale. When the current locale is not - supported, decoding input method strings becomes undefined. */ - if (XSupportsLocale ()) - xim_initialize (dpyinfo, resource_name); -#endif - xsettings_initialize (dpyinfo); /* This is only needed for distinguishing keyboard and process input. */ @@ -30832,25 +30859,33 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name) XSynchronize (dpyinfo->display, True); } +#ifdef HAVE_X_I18N { AUTO_STRING (useXIM, "useXIM"); AUTO_STRING (UseXIM, "UseXIM"); Lisp_Object value = gui_display_get_resource (dpyinfo, useXIM, UseXIM, Qnil, Qnil); + + /* `USE_XIM' controls whether Emacs should use X input methods by + default, not whether or not XIM is available. */ + #ifdef USE_XIM + dpyinfo->use_xim = true; + if (STRINGP (value) && (!strcmp (SSDATA (value), "false") || !strcmp (SSDATA (value), "off"))) - use_xim = false; -#else + dpyinfo->use_xim = false; +#else /* !USE_XIM */ + dpyinfo->use_xim = false; + if (STRINGP (value) && (!strcmp (SSDATA (value), "true") || !strcmp (SSDATA (value), "on"))) - use_xim = true; -#endif + dpyinfo->use_xim = true; +#endif /* USE_XIM */ } -#ifdef HAVE_X_I18N { AUTO_STRING (inputStyle, "inputStyle"); AUTO_STRING (InputStyle, "InputStyle"); @@ -30872,10 +30907,19 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name) #ifdef USE_GTK else if (!strcmp (SSDATA (value), "native")) dpyinfo->prefer_native_input = true; -#endif +#endif /* HAVE_GTK */ } } -#endif + + /* Now that defaults have been set up, initialize input method + support. */ + + /* Avoid initializing input methods if the X library does not + support Emacs's locale. When the current locale is not + supported, decoding input method strings becomes undefined. */ + if (XSupportsLocale ()) + xim_initialize (dpyinfo, resource_name); +#endif /* HAVE_X_I18N */ #ifdef HAVE_X_SM /* Only do this for the very first display in the Emacs session. @@ -31268,14 +31312,22 @@ x_delete_terminal (struct terminal *terminal) #ifdef HAVE_X_I18N /* We must close our connection to the XIM server before closing the X display. */ - if (dpyinfo->xim) - xim_close_dpy (dpyinfo); + xim_close_dpy (dpyinfo); #endif + /* Destroy all bitmap images created on the display. */ + image_destroy_all_bitmaps (dpyinfo); + + /* Free the storage allocated to hold bitmap records. */ + xfree (dpyinfo->bitmaps); + + /* In case someone decides to use `bitmaps' again... */ + dpyinfo->bitmaps = NULL; + dpyinfo->bitmaps_last = 0; + /* Normally, the display is available... */ if (dpyinfo->display) { - image_destroy_all_bitmaps (dpyinfo); XSetCloseDownMode (dpyinfo->display, DestroyAll); /* Delete the scratch cursor GC, should it exist. */ diff --git a/src/xterm.h b/src/xterm.h index 28ae00ca190..34a713ea2ca 100644 --- a/src/xterm.h +++ b/src/xterm.h @@ -649,7 +649,11 @@ struct x_display_info /* The named coding system to use for this input method. */ Lisp_Object xim_coding; -#endif + + /* Whether or not X input methods should be used on this + display. */ + bool use_xim; +#endif /* HAVE_X_I18N */ /* A cache mapping color names to RGB values. */ struct color_name_cache_entry **color_names; @@ -922,11 +926,6 @@ struct x_display_info #endif }; -#ifdef HAVE_X_I18N -/* Whether or not to use XIM if we have it. */ -extern bool use_xim; -#endif - #ifdef HAVE_XINPUT2 /* Defined in xmenu.c. */ extern int popup_activated_flag; -- 2.39.2