Bug 859879

Hi Alam,

To enable ibus for Assamese input, you need to do the following:

1. Go to System Settings -> Region and language

2. Click on 'Input Sources' tab, click on '+' sign to add a new input source.

3. Add all the Assamese input methods one by one or any specific Assamese input method at your wish.

4. Once you are done, under the same 'Input Sources' tab, on the right hand side of the window, you will find a hyperlink 'Shortcut Settings'. Click on it.

5. Go to 'Typing' option, click on 'Switch to previous source' on the right hand side and press 'ctrl + Backspace' to set the new shortcut for switching IM.

6. Reboot the machine and repeat the steps mentioned under 'Steps to Reproduce' of this bug.

N.B: 1. Make sure your system has Assamese language support installed before performing the above mentioned steps.

2. You won't be able to change the preferences of ibus using 'im-chooser', it is possible only in fallback mode. So, the above mentioned steps is the only way to enable ibus with the input methods of your choice.

Hope, you will be able to reproduce this bug now.

Regards,
Nilamdyuti

I could reproduce the bug.

Program received signal SIGABRT, Aborted.

#0  0x0000003070235ba5 in raise () from /lib64/libc.so.6
#1  0x0000003070237358 in abort () from /lib64/libc.so.6
#2  0x0000003c8227bb81 in g_assertion_message (domain=0x336063ad58 "IBUS", 
    file=0x336063ad45 "ibusinputcontext.c", line=863, func=
    0x336063b4c0 <__PRETTY_FUNCTION__.21585> "ibus_input_context_process_key_event_async", message=
    0x267d870 "assertion failed: (IBUS_IS_INPUT_CONTEXT (context))")
    at gtestutils.c:1848
#3  0x0000003c8227bbe2 in g_assertion_message_expr (domain=
    0x336063ad58 "IBUS", file=0x336063ad45 "ibusinputcontext.c", line=863, 
    func=
    0x336063b4c0 <__PRETTY_FUNCTION__.21585> "ibus_input_context_process_key_event_async", expr=0x336063af38 "IBUS_IS_INPUT_CONTEXT (context)")
    at gtestutils.c:1859
#4  0x000000336061f684 in ibus_input_context_process_key_event_async (context=
    0x2266700, keyval=65507, keycode=29, state=1073750021, timeout_msec=-1, 
    cancellable=0x0, callback=0x7f99d45a0f12 <_process_key_event_done>, 
    user_data=0x258f340) at ibusinputcontext.c:863
#5  0x00007f99d45a137a in _key_snooper_cb (widget=0x21ba090, event=0x258f100, 
    user_data=0x0) at ibusimcontext.c:393
#6  0x0000003c84da20e4 in gtk_main_do_event () from /lib64/libgtk-3.so.0
#7  0x0000003c83a4e1d2 in gdk_event_source_dispatch ()
   from /lib64/libgdk-3.so.0

Created attachment 617482 [details]
Patch for ibusimcontext.c

The following is the tentative patch.
IBusIMContext is destroyed during calling _request_surrounding_text.

Thanks for looking into this, but I'm a bit curious that why "context" is not NULL in the backtrace.  Isn't it cleared in _ibus_context_destroy_cb, even though there might be a race condition between g_object_unref and NULL assignment.

Also perhaps the last condition:

if (!IBUS_IS_IM_CONTEXT (ibusimcontext))

should be:

if (!IBUS_IS_INPUT_CONTEXT (ibusimcontext->context))

?

Sorry if I'm missing something.

(In reply to comment #5)
> Thanks for looking into this, but I'm a bit curious that why "context" is
> not NULL in the backtrace.  Isn't it cleared in _ibus_context_destroy_cb,
> even though there might be a race condition between g_object_unref and NULL
> assignment.

No, no, it's not _ibus_context_destroy_cb but ibus_im_context_finalize .

> 
> Also perhaps the last condition:
> 
> if (!IBUS_IS_IM_CONTEXT (ibusimcontext))
> 
> should be:
> 
> if (!IBUS_IS_INPUT_CONTEXT (ibusimcontext->context))


AFAIK, the last function does not need ibusimcontext->context .

(In reply to comment #6)
> No, no, it's not _ibus_context_destroy_cb but ibus_im_context_finalize .

Ah, ok I see.  But it looks a bit surprising to me that we need to check if the object itself is being finalized in its virtual methods (focus_in and filter_keypress).  Perhaps this problem might be a regression caused by:
https://github.com/ibus/ibus/commit/8166b0b98bfe0d70c03fa26bb5e721a357ad4ef8 ?

(In reply to comment #7)
> filter_keypress).  Perhaps this problem might be a regression caused by:
> https://github.com/ibus/ibus/commit/8166b0b98bfe0d70c03fa26bb5e721a357ad4ef8
> ?

My build does not include that patch.

Environment:
 - gnome-shell
 - sources are [('xkb', 'jp'), ('ibus', 'm17n:as:inscript'), ('ibus', 'm17n:as:inscript2'), ('ibus', 'anthy')]
 - switching engines in about 1 to 3 minutes.

(In reply to comment #8)
> (In reply to comment #7)
> > filter_keypress).  Perhaps this problem might be a regression caused by:
> > https://github.com/ibus/ibus/commit/8166b0b98bfe0d70c03fa26bb5e721a357ad4ef8
> > ?
> 
> My build does not include that patch.

Well, really?

$ fedpkg clone ibus
$ cd ibus
$ fedpkg switch-branch f18
$ fedpkg srpm
...
Wrote: /home/ueno/cvs/ibus/ibus-1.4.99.20120914-2.fc19.src.rpm
$ quilt setup ibus.spec
$ cd ibus-1.4.99.20120914
$ quilt push -a
$ grep gdk_threads_add_idle_full client/gtk2/*
client/gtk2/ibusimcontext.c:    gdk_threads_add_idle_full (G_PRIORITY_DEFAULT_IDLE,

> Environment:
>  - gnome-shell
>  - sources are [('xkb', 'jp'), ('ibus', 'm17n:as:inscript'), ('ibus',
> 'm17n:as:inscript2'), ('ibus', 'anthy')]
>  - switching engines in about 1 to 3 minutes.

Sorry, couldn't reproduce it here with:
http://dl.fedoraproject.org/pub/alt/qa/18/20120818_i18n-testday/
It includes ibus-1.4.99.20120914-2.

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > filter_keypress).  Perhaps this problem might be a regression caused by:
> > > https://github.com/ibus/ibus/commit/8166b0b98bfe0d70c03fa26bb5e721a357ad4ef8
> > > ?
> > 
> > My build does not include that patch.
> 
> Well, really?

I mean I use the old ibus.

Probably you need to use both Ctrl+space and Ctrl+Shift+space.

Now I noticed an interested result:

#0  ibus_im_context_finalize (obj=0x2aa90b0) at ibusimcontext.c:647
#1  0x0000003c82a148cb in g_object_unref () from /lib64/libgobject-2.0.so.0
#2  0x0000003c82a34b13 in g_value_unset () from /lib64/libgobject-2.0.so.0
#3  0x0000003c82a297a1 in g_signal_emit_valist ()
   from /lib64/libgobject-2.0.so.0
#4  0x0000003c82a29942 in g_signal_emit () from /lib64/libgobject-2.0.so.0
#5  0x00007f87eae1b094 in _request_surrounding_text (context=0x2aa90b0)
    at ibusimcontext.c:278
#6  0x00007f87eae1b4b8 in _key_snooper_cb (widget=0x25a6090, event=0x296a070, 
    user_data=0x0) at ibusimcontext.c:380
#7  0x0000003c84da20e4 in gtk_main_do_event () from /lib64/libgtk-3.so.0
#8  0x0000003c83a4e1d2 in gdk_event_source_dispatch ()
   from /lib64/libgdk-3.so.0
#9  0x0000003c82251722 in g_main_dispatch (context=0x245f910) at gmain.c:2707

(In reply to comment #10)
> 
> #0  ibus_im_context_finalize (obj=0x2aa90b0) at ibusimcontext.c:647
> #1  0x0000003c82a148cb in g_object_unref () from /lib64/libgobject-2.0.so.0
> #2  0x0000003c82a34b13 in g_value_unset () from /lib64/libgobject-2.0.so.0
> #3  0x0000003c82a297a1 in g_signal_emit_valist ()
>    from /lib64/libgobject-2.0.so.0
> #4  0x0000003c82a29942 in g_signal_emit () from /lib64/libgobject-2.0.so.0
> #5  0x00007f87eae1b094 in _request_surrounding_text (context=0x2aa90b0)
>     at ibusimcontext.c:278
> #6  0x00007f87eae1b4b8 in _key_snooper_cb (widget=0x25a6090,
> event=0x296a070, 
>     user_data=0x0) at ibusimcontext.c:380
> #7  0x0000003c84da20e4 in gtk_main_do_event () from /lib64/libgtk-3.so.0
> #8  0x0000003c83a4e1d2 in gdk_event_source_dispatch ()
>    from /lib64/libgdk-3.so.0
> #9  0x0000003c82251722 in g_main_dispatch (context=0x245f910) at gmain.c:2707

In case the return value in the signal function is not void, it seems the signal function calls g_object_unref.
In my test, the GObject->ref_count is normally greater than 1 in the non-void signal function.
But GOIbject->ref_count is 1 in this bug, the finalize function is called.

I don't know where GObject->ref_count is increased.
I'm not sure but currently gtk_key_snooper_install is a deprecated function.

I can reproduce this problem in both gnome-shell and non-gnome-shell.

(In reply to comment #11)
> (In reply to comment #10)
> > 
> > #0  ibus_im_context_finalize (obj=0x2aa90b0) at ibusimcontext.c:647

> 
> In case the return value in the signal function is not void, it seems the
> signal function calls g_object_unref.

I think it is possible. g_signal_email may call g_object_ref/unref before and after calling the signal handlers. And in handlers, the object may be unrefed.
So seems the object is destroyed by g_singal_emit, but actually it is destroyed in a handler.


BTW, I can not reproduce it in fedora 17 with latest ibus.

BTW, Did anyone reproduce it with other applications instead of gedit?

(In reply to comment #12)
> I think it is possible. g_signal_email may call g_object_ref/unref before
> and after calling the signal handlers. And in handlers, the object may be
> unrefed.

OK, g_value_set_instance calls ref.

void
g_signal_emit_valist (gpointer instance,
                      guint    signal_id,
                      GQuark   detail,
                      va_list  var_args)
{
...
  g_value_set_instance (instance_and_params, instance);

...
      signal_emit_unlocked_R (node, detail, instance, &return_value, instance_and_params);
...
  g_value_unset (instance_and_params);
}

The problem might be caused by calling g_signal_emit_valist recursively.

> 
> 
> BTW, I can not reproduce it in fedora 17 with latest ibus.

I cannot reproduce this in f17. Probably the switching may need to be handled by gnome-settings-daemon instead of ibus gtk panel.


(In reply to comment #13)
> BTW, Did anyone reproduce it with other applications instead of gedit?

xchat-gnome too.

Created attachment 618516 [details]
Patch for gtkimmulticontext.c

I got the following stack:

#0  gtk_im_multicontext_set_slave (multicontext=0x1e8c330 [GtkIMMulticontext], 
    slave=0x0, finalizing=0) at gtkimmulticontext.c:211
#1  0x00007f5f0b428f9b in gtk_im_multicontext_get_slave (multicontext=
    0x1e8c330 [GtkIMMulticontext]) at gtkimmulticontext.c:293
#2  0x00007f5f0b429532 in gtk_im_multicontext_set_surrounding (context=
    0x1e8c330 [GtkIMMulticontext], text=0x200c0b0 "", len=0, cursor_index=0)
    at gtkimmulticontext.c:519
#3  0x00007f5f0b425333 in gtk_im_context_set_surrounding (context=
    0x1e8c330 [GtkIMMulticontext], text=0x200c0b0 "", len=0, cursor_index=0)
    at gtkimcontext.c:663
#4  0x00007f5f0b55bc06 in gtk_text_view_retrieve_surrounding_handler (context=
    0x1e8c330 [GtkIMMulticontext], text_view=0x1eb40d0 [GeditView])
    at gtktextview.c:8146
#5  0x00007f5f0b44cc09 in _gtk_marshal_BOOLEAN__VOID (closure=0x1eb3700, 
    return_value=0x7fff1f8300c0, n_param_values=1, param_values=
    0x7fff1f8301f0, invocation_hint=0x7fff1f8300f0, marshal_data=0x0)
    at gtkmarshalers.c:2003
#6  0x00007f5f0ad923db in g_closure_invoke (closure=0x1eb3700, return_value=
    0x7fff1f8300c0, n_param_values=1, param_values=0x7fff1f8301f0, 
    invocation_hint=0x7fff1f8300f0) at gclosure.c:788
#7  0x00007f5f0adb02c3 in signal_emit_unlocked_R (node=0x1e21000, detail=0, 
    instance=0x1e8c330, emission_return=0x7fff1f8302c0, instance_and_params=
    0x7fff1f8301f0) at gsignal.c:3591
#8  0x00007f5f0adaf24a in g_signal_emit_valist (instance=0x1e8c330, signal_id=
    372, detail=0, var_args=0x7fff1f8304f0) at gsignal.c:3326
#9  0x00007f5f0adafab8 in g_signal_emit_by_name (instance=0x1e8c330, 
    detailed_signal=0x7f5f0b6fe602 "retrieve-surrounding") at gsignal.c:3425
#10 0x00007f5f0b429631 in gtk_im_multicontext_retrieve_surrounding_cb (slave=
    0x21126b0 [IBusIMContext], multicontext=0x1e8c330 [GtkIMMulticontext])
    at gtkimmulticontext.c:560
#11 0x00007f5f0b44cc09 in _gtk_marshal_BOOLEAN__VOID (closure=0x2119690, 
    return_value=0x7fff1f8307e0, n_param_values=1, param_values=
    0x7fff1f830910, invocation_hint=0x7fff1f830810, marshal_data=0x0)
    at gtkmarshalers.c:2003
#12 0x00007f5f0ad923db in g_closure_invoke (closure=0x2119690, return_value=
    0x7fff1f8307e0, n_param_values=1, param_values=0x7fff1f830910, 
    invocation_hint=0x7fff1f830810) at gclosure.c:788
#13 0x00007f5f0adb02c3 in signal_emit_unlocked_R (node=0x1e21000, detail=0, 
    instance=0x21126b0, emission_return=0x7fff1f8309e0, instance_and_params=
    0x7fff1f830910) at gsignal.c:3591
#14 0x00007f5f0adaf24a in g_signal_emit_valist (instance=0x21126b0, signal_id=
    372, detail=0, var_args=0x7fff1f830c18) at gsignal.c:3326
#15 0x00007f5f0adaf95a in g_signal_emit (instance=0x21126b0, signal_id=372, 
    detail=0) at gsignal.c:3388
#16 0x00007f5eeda90094 in _request_surrounding_text (context=
    0x21126b0 [IBusIMContext]) at ibusimcontext.c:278
#17 0x00007f5eeda904da in _key_snooper_cb (widget=0x1b7a020 [GeditWindow], 
    event=0x1dc56c0, user_data=0x0) at ibusimcontext.c:381
#18 0x00007f5f0b448a45 in gtk_invoke_key_snoopers (grab_widget=
    0x1b7a020 [GeditWindow], event=0x1dc56c0) at gtkmain.c:2259
#19 0x00007f5f0b4478a9 in gtk_main_do_event (event=0x1dc56c0) at gtkmain.c:167


When 'retrieve-surrounding' signal is emitted, gtk_im_multicontext_get_slave() is called and if priv->context_id and _gtk_im_module_get_default_context_id() are different values, the priv->slave is unref.

There is a timing issue to have the unique value between priv->context_id and _gtk_im_module_get_default_context_id().

and then g_signal_emit_valist() calls ibus_im_context_finalize() in this bug.

The tentative patch is attached.

This problem would be caused by the following patch:
http://git.gnome.org/browse/gtk+/commit/?id=a0f155e83938f6c3e63c312107dee2a970c2eb15

I can't reproduce this here with the latest upstream ibus and gtk+.

After reading this it's also not clear to me what's happening. If the problem is that gtk+ is unreffing the ibus context while the ibus module is doing something with that context then isn't it the responsibility of the ibus module to make sure that the context is reffed so that it doesn't go away if the module still needs it?

Created attachment 619736 [details]
Patch for gtkimmulticontext.c

I updated the patch.

The problem is whether the slaves need to be updated in every GtkIMMultiContext or not when the global_context_id is updated.
GtkIMContext have had the local state but it seems you are changing to have one global state.

If the global_context_id status is used, I think the slave needs to be unref when 'notify::gtk-im-module' is received so that the slave will call the finalized API before 'retrieve-surrounding' is emitted.

(In reply to comment #17)
> I can't reproduce this here with the latest upstream ibus and gtk+.
> 

Probably you need to use some m17n engines which uses the surrounding feature.
I can reproduce this problem with m17n:as:inscript and m17n:as:inscript2 .

We should really stop using the key snooper. What is its usefulness again? If I run with IBUS_DISABLE_SNOOPER=1 nothing seems to break.

Anyway, I still can't reproduce this but I can see how it may happen by looking at the code.

Basically _key_snooper_cb() relies on _focus_im_context staying alive which might not happen since the object isn't owned by this code. If it should be owned then it must be referenced at some point. Maybe something like:

diff --git a/client/gtk2/ibusimcontext.c b/client/gtk2/ibusimcontext.c
index cb5561b..72de2d0 100644
--- a/client/gtk2/ibusimcontext.c
+++ b/client/gtk2/ibusimcontext.c
@@ -373,34 +373,38 @@ _key_snooper_cb (GtkWidget   *widget,
     } while (0);
 
     if (ibusimcontext != NULL) {
+        guint state = event->state;
+
+        g_object_ref (ibusimcontext);
         _request_surrounding_text (ibusimcontext);
         ibusimcontext->time = event->time;
-    }
 
-    guint state = event->state;
-    if (event->type == GDK_KEY_RELEASE) {
-        state |= IBUS_RELEASE_MASK;
-    }
+        if (event->type == GDK_KEY_RELEASE) {
+            state |= IBUS_RELEASE_MASK;
+        }
 
-    if (_use_sync_mode) {
-        retval = ibus_input_context_process_key_event (
-                                        ibuscontext,
-                                        event->keyval,
-                                        event->hardware_keycode - 8,
-                                        state);
-    }
-    else {
-        ibus_input_context_process_key_event_async (
-                                        ibuscontext,
-                                        event->keyval,
-                                        event->hardware_keycode - 8,
-                                        state,
-                                        -1,
-                                        NULL,
-                                        _process_key_event_done,
-                                        gdk_event_copy ((GdkEvent *) event));
-        retval = TRUE;
+        if (_use_sync_mode) {
+            retval = ibus_input_context_process_key_event (
+                                                           ibuscontext,
+                                                           event->keyval,
+                                                           event->hardware_keycode - 8,
+                                                           state);
+        }
+        else {
+            ibus_input_context_process_key_event_async (
+                                                        ibuscontext,
+                                                        event->keyval,
+                                                        event->hardware_keycode - 8,
+                                                        state,
+                                                        -1,
+                                                        NULL,
+                                                        _process_key_event_done,
+                                                        gdk_event_copy ((GdkEvent *) event));
+            retval = TRUE;
+
+        }
 
+        g_object_unref (ibusimcontext);
     }
 
     if (retval) {

(In reply to comment #20)
> We should really stop using the key snooper. What is its usefulness again?
> If I run with IBUS_DISABLE_SNOOPER=1 nothing seems to break.
> 

input methods need to receive the key events prior to each application.
I don't think IBUS_DISABLE_SNOOPER=1 since it has a problem in ibus-hangul.

> Basically _key_snooper_cb() relies on _focus_im_context staying alive which
> might not happen since the object isn't owned by this code. If it should be
> owned then it must be referenced at some point. Maybe something like:

As I think the root cause is that priv->context_id and get_effective_context_id (multicontext) are not sync, but basically I agree to ref _focus_im_context during the snooper to avoid the unexpected unref.

I follow your suggestion.

Created attachment 620096 [details]
Patch for ibusimcontext.c

The tentative patch is attached.

(In reply to comment #22)
> Created attachment 620096 [details]
> Patch for ibusimcontext.c
> 
> The tentative patch is attached.

If it fixes the crash for you I'd say go for it.

(In reply to comment #21)
> > Basically _key_snooper_cb() relies on _focus_im_context staying alive which
> > might not happen since the object isn't owned by this code. If it should be
> > owned then it must be referenced at some point. Maybe something like:
> 
> As I think the root cause is that priv->context_id and
> get_effective_context_id (multicontext) are not sync

They are not in sync because they don't really have to be, i.e. the change is done lazily just when it needs to be done. If we make them be in sync as in your previous patch, all gtk+ applications would be doing the same work at the same time (when the GtkSetting changes) which would cause spikes of CPU usage that we can otherwise avoid.

(In reply to comment #20)
> We should really stop using the key snooper. What is its usefulness again?
> If I run with IBUS_DISABLE_SNOOPER=1 nothing seems to break.
AFAIK, you will see all japanese input method (or all im using "Tab") break in gedit.. and there will be more.

Until this: https://bugzilla.gnome.org/show_bug.cgi?id=90082 is fixed, no im can take risk for breaking so many things.

BTW, is using weak_pointer to surround _request_surrounding_text a better idea here?

(In reply to comment #23)
> (In reply to comment #21)
> > > Basically _key_snooper_cb() relies on _focus_im_context staying alive which
> > > might not happen since the object isn't owned by this code. If it should be
> > > owned then it must be referenced at some point. Maybe something like:
> > 
> > As I think the root cause is that priv->context_id and
> > get_effective_context_id (multicontext) are not sync
> 
> They are not in sync because they don't really have to be, i.e. the change
> is done lazily just when it needs to be done. If we make them be in sync as
> in your previous patch, all gtk+ applications would be doing the same work
> at the same time (when the GtkSetting changes) which would cause spikes of
> CPU usage that we can otherwise avoid.

OK, make sense.

https://github.com/ibus/ibus/commit/e97056e15badac9c0578a89d0085bfcb57ff7a2e

ibus-1.4.99.20121006-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/ibus-1.4.99.20121006-1.fc18

ibus-1.4.99.20121006-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/ibus-1.4.99.20121006-1.fc17

Package ibus-1.4.99.20121006-1.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing ibus-1.4.99.20121006-1.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-15547/ibus-1.4.99.20121006-1.fc18
then log in and leave karma (feedback).

ibus-1.4.99.20121006-2.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.