Errors on TSan bot after Mojo associated interfaces change |
|||
Issue descriptionThere are a few errors being reported on the TSan bot. These errors coincide with a mojo change. The errors started in this build: https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20TSan%20Tests/builds/22805 This build also contains this change https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20TSan%20Tests/builds/22805, "Support early associated interface binding on ChannelMojo" The errors disappeared in this build: https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20TSan%20Tests/builds/22816 That build reverted the Mojo change. The errors appeared again in this build: https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20TSan%20Tests/builds/22837 That build relanded the Mojo change. The errors appear kinda unrelated to the Mojo change, except that they use IPC. Nothing else in the original build seems like it could have caused the errors. Ken, any ideas? I tried to revert your change but there are conflicts :/
,
Jul 26 2016
BTW the errors are all races. It could be that there are existing races that the Mojo change is making the tests reveal, but I couldn't see anything obvious (to me).
,
Jul 26 2016
,
Jul 26 2016
The change in question completes IPC channel setup earlier than it was completed before. This could very well be exposing data races in other parts of the code. This seems very likely given that the failures are a relatively small, fixed set of tests - service worker tests and one interstitial test. I haven't looked at the tests closely, but given the stack traces it seems extremely likely that the tests are adding a MessageFilter to an IPC channel (or modifying the state of a MessageFilter in an unguarded manner) after the channel has already been connected. This is incorrect behavior. I'm happy to take a look at this if you like, but I don't think it would be right to revert my change. If you want to green the bots I think we should disable the tests for now.
,
Jul 26 2016
OK will file separate bugs for the SW / Interstitial tests.
,
Jul 26 2016
,
Aug 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/45268ac16ef424a359dbc5d130a2225dd7f13368 commit 45268ac16ef424a359dbc5d130a2225dd7f13368 Author: shimazu <shimazu@chromium.org> Date: Mon Aug 08 04:48:14 2016 ServiceWorker: Run functions in the browsertest on appropriate thread This patch fixes some race conditions which are exposed by fixes of mojo; previously some variables (|quit_| and |messages_|) were shared among IO thread and UI thread in race condition, and EmbeddedWorkerInstance::AddListener/RemoveListener was called on UI thread. TSan failures caused by these problems will be cleared by this patch. BUG= 631323 , 631311 TEST=/out/tsan/content_browsertests --no-sandbox --gtest_filter="ServiceWorkerVersionBrowserTest.*" Review-Url: https://codereview.chromium.org/2204873002 Cr-Commit-Position: refs/heads/master@{#410300} [modify] https://crrev.com/45268ac16ef424a359dbc5d130a2225dd7f13368/content/browser/service_worker/service_worker_browsertest.cc |
|||
►
Sign in to add a comment |
|||
Comment 1 by benwells@chromium.org
, Jul 26 2016