Linux: Disable DBus auto-launch

Linux: Disable DBus auto-launch

This is a workaround (ETA ~ 2-3 years) for libdbus not being multi-threading
friendly and causing random hangs when running chrome outside of Linux
desktop environments.

Background:
-----------
Typically, Linux desktop environments set the DBUS_SESSION_BUS_ADDRESS
environment variable. This variable allows the dbus client library to
directly connect to the existing bus, which is started by the desktop
environment or systemd.
When this variable is missing, the dbus client library will fallback
to auto-launch mode [1], which causes 4 nested fork() + exec() calls.
Doing this has two problems: (i) slows down startup; (ii) can hang
the browser if the fork() happens while another thread is in a malloc()
(Chrome's tcmalloc has no at-fork handlers).
This situation (no env variable) is very common in test scenarios
(browsertests, chromedriver, etc).

Change introduced by this CL:
-----------------------------
This CL sets the bus address env variable to "disabled:" if not set.
This effectively shuts down the dbus auto-launch. If necessary, this
behavior can be restored by setting, before launching chrome,
DBUS_SESSION_BUS_ADDRESS="autolaunch:" .
This workaround will be necessary until libdbus and gspawn are fixed
to be multi-threading friendly [2,3] and that fix rolls into the
various distributions.
The change is introduced in the main embedder rather than in the
google-chrome wrapper, as several binaries can be affected by this,
for instance:
- browser tests (http://crbug.com/693668)
- chrome --headless
- webdriver/selenium which seem to directly invoke "chrome"
   see https://github.com/SeleniumHQ/docker-selenium/issues/87

[1] https://dbus.freedesktop.org/doc/dbus-launch.1.html
[2] https://bugs.freedesktop.org/show_bug.cgi?id=100843
[3] https://bugs.chromium.org/p/chromedriver/issues/detail?id=1699

BUG=715658, 695643, 713947
TEST=strace -ff -o trace chrome; grep dbus-launch trace*

Review-Url: https://codereview.chromium.org/2861163002
Cr-Commit-Position: refs/heads/master@{#469987}
Committed: https://chromium.googlesource.com/chromium/src/+/8511820ec8280caacbd4f81f3ecd13b6c61681b0

[Primiano Tucci (use gerrit), 2017-05-05 13:21:54]

Description was changed from

==========
Linux: Disable DBus auto-launch

Background:
-----------
Typically, Linux desktop environments set the DBUS_SESSION_BUS_ADDRESS
environment variable. This variable allows the dbus client library to
directly connect to the existing bus, which is started by the desktop
environment or systemd.
When this variable is missing, the dbus client library will fallback
to auto-launch mode [1], which causes 4 nested fork() + exec() calls.
Doing this has two problems: (i) slows down startup; (ii) can hang
the browser if the fork() happens while another thread is in a malloc()
(Chrome's tcmalloc has no at-fork handlers).
This situation (no env variable) is very common in test scenarios
(browsertests, chromedriver, etc).

Change introduced by this CL:
-----------------------------
This CL sets the bus address env variable to "disabled:" if not set.
This effectively shuts down the dbus auto-launch. If necessary, this
behavior can be restored by setting, before launching chrome,
DBUS_SESSION_BUS_ADDRESS="autolaunch:" .
This workaround will be necessary until libdbus and gspawn are fixed
to be multi-threading friendly [2,3] and that fix rolls into the
various distributions.

[1] https://dbus.freedesktop.org/doc/dbus-launch.1.html
[2] https://bugs.freedesktop.org/show_bug.cgi?id=100843
[3] https://bugs.chromium.org/p/chromedriver/issues/detail?id=1699

BUG=715658,695643,713947
TEST=strace -ff -o trace chrome; grep dbus-launch trace*
==========

to

==========
Linux: Disable DBus auto-launch

This is a workaround (ETA ~ 2-3 years) for libdbus not being multi-threading
friendly and causing random hangs when running chrome outside of Linux
desktop environments.

Background:
-----------
Typically, Linux desktop environments set the DBUS_SESSION_BUS_ADDRESS
environment variable. This variable allows the dbus client library to
directly connect to the existing bus, which is started by the desktop
environment or systemd.
When this variable is missing, the dbus client library will fallback
to auto-launch mode [1], which causes 4 nested fork() + exec() calls.
Doing this has two problems: (i) slows down startup; (ii) can hang
the browser if the fork() happens while another thread is in a malloc()
(Chrome's tcmalloc has no at-fork handlers).
This situation (no env variable) is very common in test scenarios
(browsertests, chromedriver, etc).

Change introduced by this CL:
-----------------------------
This CL sets the bus address env variable to "disabled:" if not set.
This effectively shuts down the dbus auto-launch. If necessary, this
behavior can be restored by setting, before launching chrome,
DBUS_SESSION_BUS_ADDRESS="autolaunch:" .
This workaround will be necessary until libdbus and gspawn are fixed
to be multi-threading friendly [2,3] and that fix rolls into the
various distributions.

[1] https://dbus.freedesktop.org/doc/dbus-launch.1.html
[2] https://bugs.freedesktop.org/show_bug.cgi?id=100843
[3] https://bugs.chromium.org/p/chromedriver/issues/detail?id=1699

BUG=715658,695643,713947
TEST=strace -ff -o trace chrome; grep dbus-launch trace*
==========

[Primiano Tucci (use gerrit), 2017-05-05 13:21:54]

primiano@chromium.org changed reviewers:
+ satorux@google.com, thestig@chromium.org

[Lei Zhang, 2017-05-05 21:50:39]

lgtm

I've definitely hit my share of these deadlocks.

[Primiano Tucci (use gerrit), 2017-05-05 23:44:27]

primiano@chromium.org changed reviewers:
+ jam@chromium.org

[Primiano Tucci (use gerrit), 2017-05-05 23:44:28]

+jam, for OWNERS

[satorux1, 2017-05-08 01:30:36]

satorux@chromium.org changed reviewers:
+ satorux@chromium.org

[satorux1, 2017-05-08 01:30:37]

https://codereview.chromium.org/2861163002/diff/1/services/service_manager/em...
File services/service_manager/embedder/main.cc (right):

https://codereview.chromium.org/2861163002/diff/1/services/service_manager/em...
services/service_manager/embedder/main.cc:351:
setenv("DBUS_SESSION_BUS_ADDRESS", "disabled:", kNoOverrideIfAlreadySet);
Several environmental variables, such as GNOME_DISABLE_CRASH_DIALOG, are set in
/opt/google/chrome/google-chrome. I'm wondering if this one could be also set in
that file. If that's not the case, might be nice to explain why, in the patch
description.

[Primiano Tucci (use gerrit), 2017-05-08 10:38:52]

Description was changed from

==========
Linux: Disable DBus auto-launch

This is a workaround (ETA ~ 2-3 years) for libdbus not being multi-threading
friendly and causing random hangs when running chrome outside of Linux
desktop environments.

Background:
-----------
Typically, Linux desktop environments set the DBUS_SESSION_BUS_ADDRESS
environment variable. This variable allows the dbus client library to
directly connect to the existing bus, which is started by the desktop
environment or systemd.
When this variable is missing, the dbus client library will fallback
to auto-launch mode [1], which causes 4 nested fork() + exec() calls.
Doing this has two problems: (i) slows down startup; (ii) can hang
the browser if the fork() happens while another thread is in a malloc()
(Chrome's tcmalloc has no at-fork handlers).
This situation (no env variable) is very common in test scenarios
(browsertests, chromedriver, etc).

Change introduced by this CL:
-----------------------------
This CL sets the bus address env variable to "disabled:" if not set.
This effectively shuts down the dbus auto-launch. If necessary, this
behavior can be restored by setting, before launching chrome,
DBUS_SESSION_BUS_ADDRESS="autolaunch:" .
This workaround will be necessary until libdbus and gspawn are fixed
to be multi-threading friendly [2,3] and that fix rolls into the
various distributions.

[1] https://dbus.freedesktop.org/doc/dbus-launch.1.html
[2] https://bugs.freedesktop.org/show_bug.cgi?id=100843
[3] https://bugs.chromium.org/p/chromedriver/issues/detail?id=1699

BUG=715658,695643,713947
TEST=strace -ff -o trace chrome; grep dbus-launch trace*
==========

to

==========
Linux: Disable DBus auto-launch

This is a workaround (ETA ~ 2-3 years) for libdbus not being multi-threading
friendly and causing random hangs when running chrome outside of Linux
desktop environments.

Background:
-----------
Typically, Linux desktop environments set the DBUS_SESSION_BUS_ADDRESS
environment variable. This variable allows the dbus client library to
directly connect to the existing bus, which is started by the desktop
environment or systemd.
When this variable is missing, the dbus client library will fallback
to auto-launch mode [1], which causes 4 nested fork() + exec() calls.
Doing this has two problems: (i) slows down startup; (ii) can hang
the browser if the fork() happens while another thread is in a malloc()
(Chrome's tcmalloc has no at-fork handlers).
This situation (no env variable) is very common in test scenarios
(browsertests, chromedriver, etc).

Change introduced by this CL:
-----------------------------
This CL sets the bus address env variable to "disabled:" if not set.
This effectively shuts down the dbus auto-launch. If necessary, this
behavior can be restored by setting, before launching chrome,
DBUS_SESSION_BUS_ADDRESS="autolaunch:" .
This workaround will be necessary until libdbus and gspawn are fixed
to be multi-threading friendly [2,3] and that fix rolls into the
various distributions.
The change is introduced in the main embedder rather than in the
google-chrome wrapper, as several binaries can be affected by this,
for instance:
- browser tests (http://crbug.com/693668)
- chrome --headless
- webdriver/selenium which seem to directly invoke "chrome"
   see https://github.com/SeleniumHQ/docker-selenium/issues/87

[1] https://dbus.freedesktop.org/doc/dbus-launch.1.html
[2] https://bugs.freedesktop.org/show_bug.cgi?id=100843
[3] https://bugs.chromium.org/p/chromedriver/issues/detail?id=1699

BUG=715658,695643,713947
TEST=strace -ff -o trace chrome; grep dbus-launch trace*
==========

[Primiano Tucci (use gerrit), 2017-05-08 10:39:11]

On 2017/05/08 01:30:37, satorux1 wrote:
>
https://codereview.chromium.org/2861163002/diff/1/services/service_manager/em...
> File services/service_manager/embedder/main.cc (right):
> 
>
https://codereview.chromium.org/2861163002/diff/1/services/service_manager/em...
> services/service_manager/embedder/main.cc:351:
> setenv("DBUS_SESSION_BUS_ADDRESS", "disabled:", kNoOverrideIfAlreadySet);
> Several environmental variables, such as GNOME_DISABLE_CRASH_DIALOG, are set
in
> /opt/google/chrome/google-chrome. I'm wondering if this one could be also set
in
> that file. If that's not the case, might be nice to explain why, in the patch
> description.

Done, see updated CL description

[jam, 2017-05-08 14:05:52]

rs lgtm

[Primiano Tucci (use gerrit), 2017-05-08 14:06:15]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2017-05-08 14:06:30]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-08 15:10:00]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1494252375145030, "parent_rev":
"458ebb250a3c591dba62174b20351b5665113acd", "commit_rev":
"8511820ec8280caacbd4f81f3ecd13b6c61681b0"}

[commit-bot: I haz the power, 2017-05-08 15:10:11]

Description was changed from

==========
Linux: Disable DBus auto-launch

This is a workaround (ETA ~ 2-3 years) for libdbus not being multi-threading
friendly and causing random hangs when running chrome outside of Linux
desktop environments.

Background:
-----------
Typically, Linux desktop environments set the DBUS_SESSION_BUS_ADDRESS
environment variable. This variable allows the dbus client library to
directly connect to the existing bus, which is started by the desktop
environment or systemd.
When this variable is missing, the dbus client library will fallback
to auto-launch mode [1], which causes 4 nested fork() + exec() calls.
Doing this has two problems: (i) slows down startup; (ii) can hang
the browser if the fork() happens while another thread is in a malloc()
(Chrome's tcmalloc has no at-fork handlers).
This situation (no env variable) is very common in test scenarios
(browsertests, chromedriver, etc).

Change introduced by this CL:
-----------------------------
This CL sets the bus address env variable to "disabled:" if not set.
This effectively shuts down the dbus auto-launch. If necessary, this
behavior can be restored by setting, before launching chrome,
DBUS_SESSION_BUS_ADDRESS="autolaunch:" .
This workaround will be necessary until libdbus and gspawn are fixed
to be multi-threading friendly [2,3] and that fix rolls into the
various distributions.
The change is introduced in the main embedder rather than in the
google-chrome wrapper, as several binaries can be affected by this,
for instance:
- browser tests (http://crbug.com/693668)
- chrome --headless
- webdriver/selenium which seem to directly invoke "chrome"
   see https://github.com/SeleniumHQ/docker-selenium/issues/87

[1] https://dbus.freedesktop.org/doc/dbus-launch.1.html
[2] https://bugs.freedesktop.org/show_bug.cgi?id=100843
[3] https://bugs.chromium.org/p/chromedriver/issues/detail?id=1699

BUG=715658,695643,713947
TEST=strace -ff -o trace chrome; grep dbus-launch trace*
==========

to

==========
Linux: Disable DBus auto-launch

This is a workaround (ETA ~ 2-3 years) for libdbus not being multi-threading
friendly and causing random hangs when running chrome outside of Linux
desktop environments.

Background:
-----------
Typically, Linux desktop environments set the DBUS_SESSION_BUS_ADDRESS
environment variable. This variable allows the dbus client library to
directly connect to the existing bus, which is started by the desktop
environment or systemd.
When this variable is missing, the dbus client library will fallback
to auto-launch mode [1], which causes 4 nested fork() + exec() calls.
Doing this has two problems: (i) slows down startup; (ii) can hang
the browser if the fork() happens while another thread is in a malloc()
(Chrome's tcmalloc has no at-fork handlers).
This situation (no env variable) is very common in test scenarios
(browsertests, chromedriver, etc).

Change introduced by this CL:
-----------------------------
This CL sets the bus address env variable to "disabled:" if not set.
This effectively shuts down the dbus auto-launch. If necessary, this
behavior can be restored by setting, before launching chrome,
DBUS_SESSION_BUS_ADDRESS="autolaunch:" .
This workaround will be necessary until libdbus and gspawn are fixed
to be multi-threading friendly [2,3] and that fix rolls into the
various distributions.
The change is introduced in the main embedder rather than in the
google-chrome wrapper, as several binaries can be affected by this,
for instance:
- browser tests (http://crbug.com/693668)
- chrome --headless
- webdriver/selenium which seem to directly invoke "chrome"
   see https://github.com/SeleniumHQ/docker-selenium/issues/87

[1] https://dbus.freedesktop.org/doc/dbus-launch.1.html
[2] https://bugs.freedesktop.org/show_bug.cgi?id=100843
[3] https://bugs.chromium.org/p/chromedriver/issues/detail?id=1699

BUG=715658,695643,713947
TEST=strace -ff -o trace chrome; grep dbus-launch trace*

Review-Url: https://codereview.chromium.org/2861163002
Cr-Commit-Position: refs/heads/master@{#469987}
Committed:
https://chromium.googlesource.com/chromium/src/+/8511820ec8280caacbd4f81f3ecd...
==========

[commit-bot: I haz the power, 2017-05-08 15:10:12]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/8511820ec8280caacbd4f81f3ecd...

[iclelland, 2017-05-08 18:40:10]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2869843003/ by iclelland@chromium.org.

The reason for reverting is: Speculative revert -- the TSan bots have been
reporting a data race when setting Envvars (in this case, appending to the
python path to start a websocket server). The race appeared immediately after
this patch landed, so it may be legitimate. Reverting this to see if it clears
the failures up; if so, we'll probably just have to serialize the calls to
setenv.

Filed crbug.com/719633 for this as well..

[Primiano Tucci (use gerrit), 2017-05-09 10:57:10]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2868023002/ by primiano@chromium.org.

The reason for reverting is: Adding TSan suppression. The race is independent of
this CL, see crbug.com/719633.

[Primiano Tucci (use gerrit), 2017-05-09 16:13:44]

FTR this relanded in https://codereview.chromium.org/2865283002/ - #470301