Another problem with gtk+/gtk/gtkimmulticontext.c

[<yaoz vidar niaaa nih gov>]

I have encountered another problem with gtk+/gtk/gtkimmulticontext.c:


Symptom
-------

I am trying to watch the life cycle of an input method, for example,
'xim' module comes with GTK+ 2.0.  Switching to 'xim' in gtk+/tests/texttext,
I see the following calling sequence:

gtk_im_context_xim_register_type
gtk_im_context_xim_new
gtk_im_context_xim_class_init
gtk_im_context_xim_init
gtk_im_context_xim_new
gtk_im_context_xim_init
...

Notice that TWO new GtkIMContextXIMs are created here.

Then switching back to 'default' input method:

gtk_im_context_tim_finalize

Now only one finalize() is called.  Clearly another one is leaked some
where.


Analysis
--------

After some debugging, I found the problem is caused by
gtk+/gtk/gtkimmulticontext.c:gtk_im_multicontext_set_slave().  The
function looks like this:

155 static void
156 gtk_im_multicontext_set_slave (GtkIMMulticontext *multicontext,
157                                GtkIMContext      *slave,
158                                gboolean           finalizing)
159 {
160   if (multicontext->slave)
161     {
162       if (!finalizing)
163         gtk_im_context_reset (multicontext->slave);
164       
165       g_signal_handlers_disconnect_by_func (multicontext->slave,
166                                             (gpointer) gtk_im_multicontext_preedit_start_cb,
167                                             multicontext);
168       g_signal_handlers_disconnect_by_func (multicontext->slave,
169                                             (gpointer) gtk_im_multicontext_preedit_end_cb,
170                                             multicontext);
171       g_signal_handlers_disconnect_by_func (multicontext->slave,
172                                             (gpointer) gtk_im_multicontext_preedit_changed_cb,
173                                             multicontext);
174       g_signal_handlers_disconnect_by_func (multicontext->slave,
175                                             (gpointer) gtk_im_multicontext_commit_cb,
176                                             multicontext);
177 
178       g_object_unref (multicontext->slave);
179       multicontext->slave = NULL;
180 
181       if (!finalizing)
182         g_signal_emit_by_name (multicontext, "preedit_changed");
183     }
184   
185   multicontext->slave = slave;
186 
187   if (multicontext->slave)
188     {
189       g_object_ref (multicontext->slave);
190 
191       g_signal_connect (multicontext->slave, "preedit_start",
192                         G_CALLBACK (gtk_im_multicontext_preedit_start_cb),
193                         multicontext);
194       g_signal_connect (multicontext->slave, "preedit_end",
195                         G_CALLBACK (gtk_im_multicontext_preedit_end_cb),
196                         multicontext);
197       g_signal_connect (multicontext->slave, "preedit_changed",
198                         G_CALLBACK (gtk_im_multicontext_preedit_changed_cb),
199                         multicontext);
200       g_signal_connect (multicontext->slave, "commit",
201                         G_CALLBACK (gtk_im_multicontext_commit_cb),
202                         multicontext);
203       g_signal_connect (multicontext->slave, "retrieve_surrounding",
204                         G_CALLBACK (gtk_im_multicontext_retrieve_surrounding_cb),
205                         multicontext);
206       g_signal_connect (multicontext->slave, "delete_surrounding",
207                         G_CALLBACK (gtk_im_multicontext_delete_surrounding_cb),
208                         multicontext);
209       
210       if (multicontext->client_window)
211         gtk_im_context_set_client_window (slave, multicontext->client_window);
212     }
213 }

While switching to another IM module,

gtk_im_multicontext_set_slave(multicontext, NULL, FALSE);

is called.  At this time, 'global_context_id' is already set to, for example,
"xim".  Guess what the following code segment will do?

179       multicontext->slave = NULL;
180 
181       if (!finalizing)
182         g_signal_emit_by_name (multicontext, "preedit_changed");
183     }
184   
185   multicontext->slave = slave;
186 

One side effect of line 182 is that a new GtkIMContextXIM is created and
assigned to multicontext->slave.  Later, line 185 simply assigns
multicontext->slave to another value (in this case, NULL).  This is
where the leak is.


Possible Fix
------------

According to log, line 182 was added to 'emit "preedit_changed"
in case there was still a preedit string'.  Which IM module will handle
this "preedit_changed" signal?  My understanding is it will be the new one we
are switching to.  Because the new IM module is to be created, it is equivalent
to clearing the preedit area.  If my understanding is right, then the fix
could be quite simple:

index: gtkimmulticontext.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkimmulticontext.c,v
retrieving revision 1.20
diff -u -r1.20 gtkimmulticontext.c
--- gtkimmulticontext.c 2002/03/20 21:47:00     1.20
+++ gtkimmulticontext.c 2002/03/21 19:21:42
@@ -177,9 +177,6 @@

       g_object_unref (multicontext->slave);
       multicontext->slave = NULL;
-
-      if (!finalizing)
-       g_signal_emit_by_name (multicontext, "preedit_changed");
     }

   multicontext->slave = slave;
@@ -209,6 +206,9 @@

       if (multicontext->client_window)
        gtk_im_context_set_client_window (slave, multicontext->client_window);
+
+      if (!finalizing)
+       g_signal_emit_by_name (slave, "preedit_changed");
     }
 }

That is, moving emit "preedit_changed" signal to the slave section after
the slave has been created.


Testing
-------

I have tested the above fix, which seems to be working as expected.


Regards,

Yao Zhang