Issue 6515 in chromium-os: ibus-daemon is not killed when X stops
chrom...@googlecode.com
Owner: ----
CC: zo...@chromium.org, yus...@chromium.org, sat...@chromium.org
Labels: Type-Bug Pri-2 Area-TextInput I18N Mstone-R9
New issue 6515 by yus...@chromium.org: ibus-daemon is not killed when X
stops
http://code.google.com/p/chromium-os/issues/detail?id=6515
Chrome OS Version : tot
What steps will reproduce the problem?
1. boot the OS
2. press ctrl+alt+F2, exec 'ps auwx | grep ibus', make sure ibus-daemon and
related processes are not running
3. login
4. enable Mozc (Japanese IME), type Japanese using Mozc
5. press ctrl+alt+F2, exec 'ps auwx | grep ibus', make sure ibus-daemon and
related processes are running.
(suppose the pid of ibus-daemon is P)
6. logout
7. on the login screen, press ctrl+alt+F2, exec 'ps auwx | grep ibus'
What is the expected output? What do you see instead?
expected:
ibus-daemon and related processes are not running.
actual:
ibus-daemon and related processes are running. The PID of ibus-daemon is P.
Please use labels and text to provide additional information.
regression??
chrom...@googlecode.com
Comment #1 on issue 6515 by yus...@chromium.org: ibus-daemon is not killed
This is bad since input methods do not work correctly on the next user
session after the step 7 above.
Chrome tries to start the second ibus-daemon since Chrome does not know
about the first ibus-daemon (whose pid is P), but it fails with "current
session already has an ibus-daemon" error.
Attached the UI log.
ibus$ grep -B 6 -A 10 "current session already has an ibus-daemon" -r .
./bus/main.c- /* check if ibus-daemon is running in this session */
./bus/main.c- if (ibus_get_address () != NULL) {
./bus/main.c- bus = ibus_bus_new ();
./bus/main.c-
./bus/main.c- if (ibus_bus_is_connected (bus)) {
./bus/main.c- if (!replace) {
./bus/main.c: g_printerr ("current session already has an
ibus-daemon.\n");
./bus/main.c- exit (-1);
./bus/main.c- }
./bus/main.c- ibus_bus_exit (bus, FALSE);
./bus/main.c- while (ibus_bus_is_connected (bus)) {
./bus/main.c- g_main_context_iteration (NULL, TRUE);
./bus/main.c- }
./bus/main.c- }
./bus/main.c- g_object_unref (bus);
./bus/main.c- bus = NULL;
./bus/main.c- }
Attachments:
ui.20100908-040808 1.4 MB
chrom...@googlecode.com
Status: Started
Owner: yusu...@chromium.org
Cc: penghu...@chromium.org
Comment #2 on issue 6515 by yus...@chromium.org: ibus-daemon is not killed
I'm going to check if /etc/init/ibus.conf, which is supposed to kill ibus
daemons when X stops, is working as intended. Possibly it stopped working
when we had removed the main script stanza from the conf file?? (see the
diff below)
It would also be better to use "--replace" ibus-daemon command line flag in
MaybeLaunchIme() to kill stale ibus-daemon. I'll work on these tomorrow.
chrom...@googlecode.com
Labels: -Pri-2 Pri-1
Comment #3 on issue 6515 by sat...@chromium.org: ibus-daemon is not killed
(No comment was entered for this change.)
chrom...@googlecode.com
Comment #4 on issue 6515 by penghu...@chromium.org: ibus-daemon is not
http://code.google.com/p/chromium-os/issues/detail?id=6515
In Linux desktop ibus-x11 will notify the ibus-daemon, when X server stops.
But it needs executing ibus-daemon with -x or --xim.
For chromeos, ibux-x11 is not necessary, because chrome does not need xim
support. So probably, chromeos should kill the daemon, when X server stops.
chrom...@googlecode.com
Comment #5 on issue 6515 by yus...@chromium.org: ibus-daemon is not killed
Yeah, in the past, /etc/init/ibus.conf was doing the job, but it seems that
it's not working right now (we need autotest for this...).
Fixing /etc/init files:
http://codereview.chromium.org/3354014
chrom...@googlecode.com
Comment #6 on issue 6515 by yus...@chromium.org: ibus-daemon is not killed
Investigated the issue a little more.
It seems that the issue could happen only when it takes long time to logout
(e.g. when the OS is started from USB stick, when a netbook in use is
slow, ...).
If logout takes longer than expected, the session manager seems to kill
Chrome by signal 6 (TERM) and then by 15 (ABORT), but the signal(s) are
sent only to the Chrome browser process and are not sent to the whole
process group members (i.e. ibus-daemon, candidate_window, ...). And, when
Chrome is terminated by such signals, the destructor in
chrome/browser/chromeos/cros/input_method_library.cc, which is supposed to
kill ibus daemons, might not be called. This is why ibus daemons remain, I
believe.
The patch at http://codereview.chromium.org/3354014 should fix the problem.
chrom...@googlecode.com
Comment #7 on issue 6515 by bugd...@chromium.org: ibus-daemon is not
killed when X stops
http://code.google.com/p/chromium-os/issues/detail?id=6515#c7
Commit: ae978ccf660d434fd94b3f8d0f24b554ec00110f
Email: yus...@chromium.org
Move post-stop script for ibus daemons from ibus.conf to ui.conf.
Since we have deleted "script ... end script" section of ibus.conf, the
post-stop script in ibus.conf is no longer working. We have to move the
script somewhere else.
BUG=chromium-os:6515
TEST=see the bug. manually checked that the script in ui.conf kills ibus
daemons.
Review URL: http://codereview.chromium.org/3354014
D ibus.conf
M ui.conf
chrom...@googlecode.com
Status: Fixed
Comment #8 on issue 6515 by yus...@chromium.org: ibus-daemon is not killed
when X stops
chrom...@googlecode.com
Comment #9 on issue 6515 by cma...@chromium.org: ibus-daemon is not killed
I don't know that this was the best fix. If the ibus processes are
actually in chrome's process group, it's a trivial change to have the
session manager send them SIGTERM as well. I'm generally against having
one upstart file have to be aware of and manage many processes, especially
if its not the thing starting the process. It makes it harder to figure
out where things are happening in the system, which is already very
difficult.
I'd prefer it if you rolled back this change, and we had the
session_manager just SIGTERM all of chrome's process group.
If that doesn't work, or perhaps in addition as a safety net, we can
reinstate ibus.conf as a "task", have it start on stopping ui, and have it
just exec the appropriate pkill command.
chrom...@googlecode.com
Status: Started
Comment #10 on issue 6515 by yus...@chromium.org: ibus-daemon is not
killed when X stops
http://code.google.com/p/chromium-os/issues/detail?id=6515
Thanks for the comment Chris. I've reopened the issue. I'll check if
sending signal to process group works fine asap.
I think what we have to do is to modify kill(child_pid, ..) in
SessionManagerService::CleanupChildren() to kill(-child_pid, ...). Is this
correct?
...and I also noticed that ibus-daemon always creates its own process group
(I should have checked the output of 'ps xj' in advance...)
I'll ask penghuang@c (the author of ibus-daemon) if it's okay to remove the
setpgid() call in ibus-daemon.
chrom...@googlecode.com
Comment #11 on issue 6515 by cma...@chromium.org: ibus-daemon is not
http://code.google.com/p/chromium-os/issues/detail?id=6515
The kill() change you mention should work, provided that you can get ibus
to be in the same process group as chrome somehow. If you can't, that'd
understandable, and I'd recommend the ibus.conf "task" approach, in
which "stopping ui" triggers a separate job that kills ibus.
chrom...@googlecode.com
Comment #12 on issue 6515 by yus...@chromium.org: ibus-daemon is not
http://code.google.com/p/chromium-os/issues/detail?id=6515
http://codereview.chromium.org/3336018 (ibus part)
http://codereview.chromium.org/3374005 (session manager part)
chrom...@googlecode.com
Comment #13 on issue 6515 by yus...@chromium.org: ibus-daemon is not
http://code.google.com/p/chromium-os/issues/detail?id=6515
Chatted with Peng, and it turned out that ibus-daemon does not work
correctly without its own process group id. ibus-daemon sometimes sends
SIGTERM to its process group to kill its sub processes, and we can't remove
the SIGTERM code in ibus-daemon since the code is necessary for the "on
demand ibus-daemon shutdown" feature for Chrome OS.
http://github.com/ibus/ibus/blob/master/bus/ibusimpl.c#L666
Let me close the code reviews above and move to the ibus.conf solution.
chrom...@googlecode.com
Comment #14 on issue 6515 by yus...@chromium.org: ibus-daemon is not
http://code.google.com/p/chromium-os/issues/detail?id=6515
http://codereview.chromium.org/3413002
http://codereview.chromium.org/3398002
chrom...@googlecode.com
Comment #15 on issue 6515 by bugd...@chromium.org: ibus-daemon is not
killed when X stops
http://code.google.com/p/chromium-os/issues/detail?id=6515#c15
Commit: 3b8abfff28c7af53f2adc09b6d1e0cef7686f55f
Email: yus...@chromium.org
Revert "Move post-stop script for ibus daemons from ibus.conf to ui.conf."
We can kill ibus daemons in ibus.conf. See 6515 for details.
This reverts commit ae978ccf660d434fd94b3f8d0f24b554ec00110f.
BUG=chromium-os:6515
TEST=none
Review URL: http://codereview.chromium.org/3413002
A ibus.conf
M ui.conf
chrom...@googlecode.com
Comment #16 on issue 6515 by bugd...@chromium.org: ibus-daemon is not
killed when X stops
http://code.google.com/p/chromium-os/issues/detail?id=6515#c16
Commit: 5a7234441e0c632e6de56dc0e9e62323bfd91525
Email: yus...@chromium.org
Add "task" to ibus.conf since the file no longer launches ibus-daemon.
Please note that we have decided to kill ibus daemons in ibus.conf rather
than session_manager since it turned out that Chrome and ibus daemons can't
have the same process group id. Please check the issue 6515 for details.
BUG=chromium-os:6515
TEST=manually tested. see the bug.
Review URL: http://codereview.chromium.org/3398002
M ibus.conf
chrom...@googlecode.com
Status: Fixed
Comment #17 on issue 6515 by yus...@chromium.org: ibus-daemon is not
killed when X stops