Apparently some bots, such as Linux GPU TSAN have TSAN stack
traces where base::Histogram::AddCount appears as just
AddCount (while other functions are fully qualified.)
So this adds to the suppression added in
https://chromium-review.googlesource.com/c/581418 to ignore
these errors.
BUG=744734
Change-Id: I1e353894193cd3992c3a965013891bcb9f9105a8
Reviewed-on: https://chromium-review.googlesource.com/583227
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Alexei Svitkine (slow) <asvitkine@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#488971}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 9773bf1cbf291511a4aef6ec782bb20a4ebc6b7d
This change:
- Does a separate pass on all histograms, validating them, before
starting to prepare their deltas. This will confirm that data is
bad already before we start processing them (i.e. verify that it's
not the processing code having some side effect).
- Counts the number of corrupted histograms to tell us if it's a single
bad value or more widespread corruption.
- Keeps track of the last histogram that someone logged something to,
so that we could have a trail of the last thing that happened before
corruption.
- Moves a previously added instrumentation call to within the correct if
block.
BUG=736675,744734
Change-Id: I4b860ad6e977c1555409bd628f9e4e5147e61654
Reviewed-on: https://chromium-review.googlesource.com/581418
Commit-Queue: Alexei Svitkine (slow) <asvitkine@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#488815}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 2e8a3a3bbec7e6f3876a33baa9124213a82371b0
This CL was also going to fix a race condition in the existing implementation when SetTaskRunner() was used but the test-only failures caused by tests having long-lived
with the status quo are proving hard than expected to fix. The race is no worse than
before by making this code sequence-friendly (and generally healthier). Will fix
race in a follow-up CL.
Of note in this CL:
- Patch set 1 is https://codereview.chromium.org/1433373003
so looking at diff from 1 might be less work (especially for tests).
- (not true anymore, postponed to race fix CL:)
The Timer's delayed task now always lives on the sequence it was started from
(and even SetTaskRunner() was used, a task is posted to it when the delay
expires instead of having the Timer's delayed task live on it -- this solves
the aforementioned race condition).
- This required adapting tests for MediaCodecLoop and UploadProgressTracker.
BUG=587199, 552633, 678592, 684640, 675631
Review-Url: https://codereview.chromium.org/2491613004
Cr-Original-Commit-Position: refs/heads/master@{#476317}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 4e844bb5f686dbd3a062c6aaf83c8a1df171a08a
This CL:
* Removes the last dependency on sanitizers:deps_no_options and replaces it
with exe_and_shlib_deps
* Merges sanitizers:deps and sanitizers:deps_no_options
* Adds a weak symbol for NaCl to override the default sanitizer options
BUG=593874
R=thakis@chromium.org,bradnelson@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_msan_rel_ng;master.tryserver.chromium.linux:linux_chromium_ubsan_rel_ng;master.tryserver.chromium.mac:mac_chromium_asan_rel_ng
Review-Url: https://codereview.chromium.org/2911513002
Cr-Original-Commit-Position: refs/heads/master@{#475093}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: f59081471b68cb0a0c484fe9c9c3b8a1f7f2d122
It's harder to see if new errors come up since this triggers
TSAN failures in so many places. Suppress for now until a there is a
solution.
BUG=695929
Change-Id: I2729c069a29ab595208f8f04695cace3877c1083
Reviewed-on: https://chromium-review.googlesource.com/503790
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Commit-Queue: Mike Bjorge <mbjorge@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#471622}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 80429170bb3de6b4639f42fc630a2d1f8b8a7117
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 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: 8511820ec8
> Review-Url: https://codereview.chromium.org/2869843003
> Cr-Commit-Position: refs/heads/master@{#470059}
> Committed: 1e78cb7863
BUG=715658,695643,713947,719633
TBR=satorux@google.com,thestig@chromium.org,jam@chromium.org
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
Introduces the beginnings of an embedder API for the Service Manager,
consisting of a single entry point (service_manager::Main) which in
turn delegates to its embedder for arbitrary initialization and
process execution logic.
This is the first of several incremental steps to remove content from
the generic process startup flow. Future patches will rework various
main entry points to go through service_manager::Main directly, rather
than going ContentMain -> service_manager::Main.
This will also allow us to introduce new process types which run the
Service Manager or arbitrary services directly without touching any
part of content.
BUG=654986
Review-Url: https://codereview.chromium.org/2613653003
Review-Url: https://codereview.chromium.org/2613653003
Cr-Original-Commit-Position: refs/heads/master@{#458331}
Committed: c6026704ff
Cr-Original-Original-Commit-Position: refs/heads/master@{#458252}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 60d92c2f3ff60c2fcc0b3cddd05738403fb1f0c3
Reason for revert:
Introduced crashes of webkit_tests on Mac.
BUG=703465
Original issue's description:
> Move some basic early process init into Service Manager
>
> Introduces the beginnings of an embedder API for the Service Manager,
> consisting of a single entry point (service_manager::Main) which in
> turn delegates to its embedder for arbitrary initialization and
> process execution logic.
>
> This is the first of several incremental steps to remove content from
> the generic process startup flow. Future patches will rework various
> main entry points to go through service_manager::Main directly, rather
> than going ContentMain -> service_manager::Main.
>
> This will also allow us to introduce new process types which run the
> Service Manager or arbitrary services directly without touching any
> part of content.
>
> BUG=654986
>
> Review-Url: https://codereview.chromium.org/2613653003
> Cr-Commit-Position: refs/heads/master@{#458252}
> Committed: c6026704ffTBR=jam@chromium.org,rockot@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=654986
Review-Url: https://codereview.chromium.org/2763883002
Cr-Original-Commit-Position: refs/heads/master@{#458278}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 1c92dd551abe4a1581981b945076719dff7b4422
Introduces the beginnings of an embedder API for the Service Manager,
consisting of a single entry point (service_manager::Main) which in
turn delegates to its embedder for arbitrary initialization and
process execution logic.
This is the first of several incremental steps to remove content from
the generic process startup flow. Future patches will rework various
main entry points to go through service_manager::Main directly, rather
than going ContentMain -> service_manager::Main.
This will also allow us to introduce new process types which run the
Service Manager or arbitrary services directly without touching any
part of content.
BUG=654986
Review-Url: https://codereview.chromium.org/2613653003
Cr-Original-Commit-Position: refs/heads/master@{#458252}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: c6026704ff452d402924ce2d88b0168baf227b1e
V8's signal handler based bounds checking solution needs to be able to
install a custom signal handler. Unfortunately, asan does not allow this
by default.
BUG=v8:5277
Review-Url: https://codereview.chromium.org/2731393003
Cr-Original-Commit-Position: refs/heads/master@{#455218}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 8e6e95fce9bb5e972c0c96bff0d0a8ea5d7f068a
I'm working on a patch to fix asan_options on windows and I faced this
two incompatibles symbols:
00D 00000000 SECT2 notype External | ?kASanDefaultSuppressions@@3PADA (char * kASanDefaultSuppressions)
01F 00000000 UNDEF notype External | _kASanDefaultSuppressions
This is caused by the way the symbols are defined (i.e. extern "C").
src/build/sanitizers/asan_suppressions.cc
char kASanDefaultSuppressions[] =
src/build/sanitizers/sanitizer_options.cc
extern "C" char kASanDefaultSuppressions[];
BUG=681027
R=eugenis@chromium.org, chrisha@chromium.com, rnk@chromium.org
Review-Url: https://codereview.chromium.org/2631753002
Cr-Original-Commit-Position: refs/heads/master@{#444764}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: c9e8aae171ee0999ed1ad766089d76cf40159fee
This CL fixes a tsan data race, which is caused by calling StopFetchingData
from different threads. It must not be allowed. Use the same thread by using
a PostTask.
BUG=673760
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng
Review-Url: https://codereview.chromium.org/2569763004
Cr-Original-Commit-Position: refs/heads/master@{#439798}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: d52b2f15ba4712b9dc4a2d08a45aa3210999dc8f
As far as I can tell, everything references the original location of
llvm-symbolizer in third_party/llvm-build/Release+Asserts/bin, so
this shouldn't be needed.
Clusterfuzz used to need this, but it stopped needing it in
https://codereview.chromium.org/2394163006/
No intentional behavior change.
BUG=none, vaguely related to 430156 and 495204
Review-Url: https://codereview.chromium.org/2145833008
Cr-Original-Commit-Position: refs/heads/master@{#436986}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: eeaccfd53c4d582d39ec7e30f19d8e4db8b4ee49
This reverts commit f54c1207ebe045798603f6484ad1f59aecad061c.
This is a speculative revert - the original problem doesn't seem to
repro anymore (per https://crbug.com/432070#c17) so let's try to remove
the (hopefully) no longer needed test suppression.
BUG=432070
Review-Url: https://codereview.chromium.org/2557903002
Cr-Original-Commit-Position: refs/heads/master@{#436765}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 9e11ad5750fb228b83db25fcd90f77488659d750
The two AssociatedGroupController implementations we have both own a lock
which is acquired during message dispatch, among other operations.
In the EDK layer a Watcher lock is also acquired further up the stack.
Because sending a message may indirectly require notifying the same
Watcher lock, it must never be true that the AssociatedGroupController's
lock is held while its pipe is written to.
This fixes the lock-order inversion resulting from the
fact that pipe control messages were being sent under lock, and removes
the associated TSAN suppression.
BUG=663557
TBR=glider@chromium.orgR=yzshen@chromium.org
Review-Url: https://codereview.chromium.org/2494483003
Cr-Original-Commit-Position: refs/heads/master@{#431331}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 58909542197b704efe10526c169e4b502f799b57
This CL destructs base::Thread and its underlying system thread before
WorkerThread::terminateAndWait returns. This is important to make sure that
the main thread calls WTF::shutdown() after ThreadSpecifics of all threads
are destructed. See 345240 for more details.
BUG=345240
Review-Url: https://codereview.chromium.org/2251903002
Cr-Original-Commit-Position: refs/heads/master@{#412735}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 30d88091658428a57d4567d72f41a8a251031f68
The race condition is expected as we're racing the thread reclaim
logic. ThreadLocalStorage happens to not synchronize anything at the
moment. Adding a lock to synchronize usage of g_tls_destructors won't
actually fix the inherent race condition. In production, it's up the
owner to make sure that any references have cleaned up before
releasing the TLS slot.
Given that ThreadLocalStorage will be getting some locks in the future,
so I expect to be able to remove the suppression once that goes
through.
BUG=638378
Review-Url: https://codereview.chromium.org/2256493002
Cr-Original-Commit-Position: refs/heads/master@{#412362}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: ad1893a4eeeb1775709005ad7df0081560fa27d3
The compiler does not include the class name in the symbol name for the
TestBody method so the suppression added in r409915 doesn't work. This
patch disables the test under TSan instead.
BUG=634383,629716
TBR=thestig@chromium.org
NOTRY=true
Review-Url: https://codereview.chromium.org/2218663003
Cr-Original-Commit-Position: refs/heads/master@{#410159}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 4ef71eefd26cd7c4f3ee5c7d4fbff9aaed2a5459
All the LKGR clusterfuzz bots build this target, so add it to allow
switching these bots to gn.
BUG=618702,542853,619086
TBR=eroman
Review-Url: https://codereview.chromium.org/2059843002
Cr-Original-Commit-Position: refs/heads/master@{#399241}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 3f7d4fc75b62ca7008642c5aea6233eade695da5
CSSValuePool was used to be static instance on main thread
only. But OffscreenCanvas in a worker requires to access
the CSS value caches in a non-main thread. This patch uses
the ThreadSpecific persistent handles to create static
CSSValuePool instances per thread when needed, and the
cleanup code is handled in ThreadState::cleanup() added by
patch https://codereview.chromium.org/1881933005.
As a result, WebKit unit tests (which does not use the
ThreadState::cleanup() as the worker thread) need to be
modified so that false positive leak errors will not be
reported.
In addition, an indirect memory leak "__strdup
/build/eglibc-3GlaMS/eglibc-2.19/string/strdup.c" is
generated in webkit unit tests; but after printing out the
full error stack trace, we observe that it eventually
originates from libfontconfig, a third_party library that
has leaks and has already been suppressed in
leak_suppression.cc. But the default stack trace is too
short on suppress this indirect memory leak; so we added
one more leak suppression underneath the libfontconfig.
BUG=599659
Review URL: https://codereview.chromium.org/1870503002
Cr-Original-Commit-Position: refs/heads/master@{#388815}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 1174eb6ddb160985b51d1d45321e24c493aa7f83
Alexei Svitkine
Fernando Serboncini
gab
thomasanderson
Mike Bjorge
primiano
jbauman
rockot
horo
maxmorin
kcwu
glider
machenbach
eholk
grt
etienneb
maksim.sisov
robliao
thakis
lukasza
benwells
brettw
haraken
reillyg
msarett
mmoroz
thestig
falken
aizatsky
sigbjornf
bungeman
inferno
xlai
hablich
oshima