GNOME Bugzilla – Bug 87482
GTK2 XIM input method does not call IMCloseHandler when exit.
Last modified: 2011-02-04 16:11:52 UTC
I'm developing an input method with IMdkit. I found that gtk2 programs do not call IMCloseHandler to close the XIM connection, this maybe lead memory leak within Input Method server.
Is the Xlib API for this XCloseIM()? I'm not really familiar with XIM internals.
Yes, it should be XCloseIM.
Not really a bug then; GTK+ typically doesn't know when the application is done; most applications simply call exit(). GTK+ won't call XCloseDisplay() either. Any input method that leaks memory in this situation is bugy; otherwise, any program that terminates abnormally (segfault, Control-C, etc.) will cause the input method to leak memory. I assume that XIM has some provision for cleaning up when the connection with the client is broken.
But there is no way to know if the client is broken. I think at least GTK+ should call XCloseIM when the program exits normally. In my experience ,most programs do that except GTK+.
GTK+ is never going to call XCloseDisplay() or similar calls on normal program termination ... there is just no point in making the programmer write the code to do that, and GTK+ itself can't no when to do it without the programmer telling it when to do so. Still, with the multihead branch, closing a display will be possible, and we will need to call XCloseIM() there.
Moving input method related bugs to input-methods component
Toshi - could you review the patch I'm attaching? The main intent of the patch is to handle gdk_display_close() properly, but it also does a lot of cleanup on the handling of GtkXIMInfo structures. There seemed to be some confusion before this patch about whether GtkXIMInfo structures were per-display or per-context (probably resulting partly from bug 95150, which was fixed recently). This patch cleans that up so there is one GtkXIMInfo object per screen. this means that we are calling XOpenIM a bit more than we need to - once per screen, rather than once per displays, but it seemed easiest to do it that way, since we needed screen-specific handling for GtkSettings. Thanks, Owen
Created attachment 11479 [details] [review] Patch adding close_display() support
Owen - sure, I'm glad to review and test the patch - but perhaps cannot do before Monday, I'm afraid.. BTW - it may depend on the version of IMdkit and also depend on the type of XIM protocol - XIM server should be able to close the connection and free the memory even if the client has no chance to call XCloseIM(). This is also true with XDestroyIC() - the app may die without calling XDestroyIC(), but XIMserver should be able to free the memory for associate XCreateIC() call. If you see otherwise, please contact to current IMdkit maintainer in li18nux.org.
Next week should be fine ... I'm not planning to do another release in the next few days. The patch doesn't try to address the problem of calling XCloseIM for normal program termination - that's not possible if the program just calls exit() [ or terminates abnormally ]; I'm assuming that the IM server can indeed clean up properly if it loses the connection to the client. What this patch is more looking at is the question of a program explicitely disconnecting from a display using gdk_display_close() [ 2.2 only ]
I'm going to go ahead and commit this; it turns out that I need to do 2.1.1 today, and I think the code in there now is going to behave badly with the fix for bug 95150. Please still do review the patch, we can back the changes out and do them differently as needed. Mon Oct 14 15:10:34 2002 Owen Taylor <otaylor@redhat.com> * modules/input/gtkimcontextxim.c: - Make GtkXIMInfo structures per-screen (they were a mix between per-display and per-context before) - Make signal connections info->settings one-per-info - Handle a GdkDisplay being closed, by destroying all XIC, calling XCloseIM on the XIM. (#87482)
Owen, info->ics is not initialized, and this causes segv error when reinitialize_all_ics is called while no ic is yet created. So I applied the patch below: retrieving revision 1.27 diff -u -r1.27 gtkimcontextxim.c --- gtkimcontextxim.c 14 Oct 2002 19:19:08 -0000 1.27 +++ gtkimcontextxim.c 15 Oct 2002 03:44:42 -0000 @@ -370,6 +370,7 @@ info->settings = NULL; info->preedit_set = 0; info->status_set = 0; + info->ics = 0; setup_im (info); But, it dies in different place. This happens even without the change you checked in today. I'll attach a function trace after this comment.
Created attachment 11544 [details] function trace of testtext - it dies at its startup
Only if the following line is commented out, testtext and the rests of gtk+ apps seem to work fine with XIM backend. /* gtk_window_set_resizable (GTK_WINDOW (status_window), FALSE); */ Can we apply this and the above ics initialization code for 2.1.1?
Forgot to say that I'm using glib-2.0 branch. Is this relevant to this g_logv crash?
Created attachment 11545 [details] [review] a patch to avoid crashing
Looks like window changed accidentally changed to status_window in Manish's cleanups. I've fixed both problems now in CVS. Tue Oct 15 09:54:54 2002 Owen Taylor <otaylor@redhat.com> * modules/input/gtkimcontextxim.c (get_im): Initialize info->ics to NULL. (Hidetoshi Tajima.) * modules/input/gtkimcontextxim.c (status_window_get): Call set_resizable on the window, not on the structure. Too late for 2.1.1, but I'll try to make sure that 2.1.2 comes out fairly quickly.
Thanks, the fix works well with testtext, gtk-demo and gnome-terminal + libzvt-i18n, (will continue to test more apps..). Also, I reviewed the diff(cvs diff -u -r1.26 gtkimcontextxim.c) and it looks correct to me. Only thing I didn't figure it out is if xim_info_display_closed() really works or not. Any easy way to manage to call it?
To test xim_info_display_closed() you can use the "Change Display" test in demos/gtk-demo/gtk-demo - Open a new display connection, move the "Dialog and Message Box" or "Text widget" demo there, move the change display dialog there type in some text, move them back, close the new display.
gtk-demo crashes when the 2nd display is closed in the case above. It also crashes when I start gtk-demo and immediately close "0:0" display connection from "Change Display. Below is stack stace, showing display and info is somehow swapped to each other. stopped in get_im at line 377 in file "gtkimcontextxim.c" 377 g_signal_connect_swapped (display, "closed", (dbx) print display display = 0x4a438 (dbx) print info info = 0xc7c98 (dbx) c stopped in xim_info_display_closed at line 305 in file "gtkimcontextxim.c" 305 open_ims = g_slist_remove (open_ims, info); (dbx) print displau dbx: "displau" is not defined in the scope `im-xim.so`gtkimcontextxim.c`xim_info_display_closed` (dbx) print display display = 0xc7c98 (dbx) print info info = 0x4a438 (dbx) c signal BUS (invalid address alignment) in xim_info_display_closed at line 311 in file "gtkimcontextxim.c" 311 set_ic_client_window (tmp_list->data, NULL, TRUE);
Hmm, really thought I had tested it at least _that_ much. Tue Oct 15 17:25:47 2002 Owen Taylor <otaylor@redhat.com> * modules/input/gtkimcontextxim.c (get_im): Don't use connect_swapped() when the function being connected has a non-swapped signature.
looks like it passed through the xim, but hit to yet another place. It crashes even with other input method backend. program terminated by signal SEGV (no mapping at the fault address) 0xff2d0010: gdk_window_queue+0x0184: ld [%g1 + 0x60], %g2 (dbx) where =>[1] gdk_window_queue(0x237558, 0x3fe38, 0x1, 0x1, 0x7, 0x3), at 0xff2d0010 [2] _gdk_windowing_window_queue_antiexpose(0x237558, 0xc3c78, 0xffbeead0, 0x1, 0x7, 0x3), at 0xff2d0084 [3] gdk_window_process_updates_internal(0x237558, 0x2, 0x2c00, 0x2f7c, 0xff01bb5c, 0xfecf1d80), at 0xff2b5a28 [4] gdk_window_process_all_updates(0x1731f8, 0x44, 0x0, 0x1, 0xf, 0x7), at 0xff2b5b80 [5] gtk_container_idle_sizer(0x90ba0, 0xff1e5b84, 0x0, 0xffbeebfc, 0x0, 0xfebad7f4), at 0xfefb4b5c [6] g_main_dispatch(0x4cc68, 0xfec14e54, 0xfec14fec, 0xfebf90f0, 0x0, 0x4cc70), at 0xfebae90c [7] g_main_context_dispatch(0x4cc68, 0x2, 0xfec14fec, 0x11, 0x3000, 0x3394), at 0xfebafbd0 [8] g_main_context_iterate(0x4cc68, 0x1, 0x1, 0x0, 0x1, 0xfec103e4), at 0xfebaffec [9] g_main_loop_run(0xc0848, 0x1800, 0x1800, 0x0, 0x3000, 0x4cc68), at 0xfebb093c [10] gtk_main(0x38408, 0x7c00, 0x7ca8, 0x0, 0x1, 0x0), at 0xff01b250 [11] main(0x7ac80, 0x7ac80, 0xb7180, 0x38958, 0x25800, 0x25800), at 0x22a34
I think the crash above has nothing to do with GTK XIM, so this can be marked fixed now and open a new bugzilla for the crash if it has not reported yet. Shall I?
It's known that display closing isn't fully working yet; generally convered under bug 85715. Usually I can succesfully close a display without any windows on it without crashing but some sequences dont' work. (Closing a display with a window on it is definitely not going to work.) Resolving this bug as FIXED, though note that the original reported problem is more WONTFIX ... under normal exit conditions GTK+ will not close XCloseDisplay() or XCloseIM().