Skip to content

Commit

Permalink
Reland of Linux: Disable DBus auto-launch (patchset #1 id:1 of https:…
Browse files Browse the repository at this point in the history
…//codereview.chromium.org/2869843003/ )

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

Original issue's description:
> Revert of Linux: Disable DBus auto-launch (patchset #1 id:1 of https://codereview.chromium.org/2861163002/ )
>
> Reason for revert:
> 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.
>
> Original issue's description:
> > 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 SeleniumHQ/docker-selenium#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

> Review-Url: https://codereview.chromium.org/2869843003
> Cr-Commit-Position: refs/heads/master@{#470059}
> Committed: https://chromium.googlesource.com/chromium/src/+/1e78cb7863da28bb3411286cdbcc4fb4510ce173

BUG=715658,695643,713947,719633
[email protected],[email protected],[email protected]

Review-Url: https://codereview.chromium.org/2865283002
Cr-Original-Commit-Position: refs/heads/master@{#470301}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 2fc330d0b93d4bfd7bd04b9fdd3102e529901f91
  • Loading branch information
primiano authored and Commit bot committed May 9, 2017
1 parent 9b4b28c commit e7a23c4
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions sanitizers/tsan_suppressions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,9 @@ char kTSanDefaultSuppressions[] =
// http://crbug.com/691029
"deadlock:libGLX.so*\n"

// http://crbug.com/719633
"race:crypto::EnsureNSSInit()\n"

// End of suppressions.
; // Please keep this semicolon.

Expand Down

0 comments on commit e7a23c4

Please sign in to comment.