Issue metadata
Sign in to add a comment
|
SecurityKeyMessageReaderImplTest.MultipleMessages is flaky |
||||||||||||||||||||
Issue descriptionFlaky test: SecurityKeyMessageReaderImplTest.MultipleMessages Sample failed build due to flakiness: https://ci.chromium.org/buildbot/chromium.memory/Linux%20TSan%20Tests/28012 Test output log: https://chromium-swarm.appspot.com/task?id=40b64da736c5cd10 Culprit (70.0% confidence): r601600 Analysis: https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVytAELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCJ-Y2hyb21pdW0ubWVtb3J5L0xpbnV4IFRTYW4gVGVzdHMvMjgwMTIvcmVtb3RpbmdfdW5pdHRlc3RzL1UyVmpkWEpwZEhsTFpYbE5aWE56WVdkbFVtVmhaR1Z5U1cxd2JGUmxjM1F1VFhWc2RHbHdiR1ZOWlhOellXZGxjdz09DAsSE01hc3RlckZsYWtlQW5hbHlzaXMYAQw Please revert the culprit, or disable the test and find the appropriate owner. If the culprit above is wrong, please file a bug using this link: https://bugs.chromium.org/p/chromium/issues/entry?status=Unconfirmed&labels=Pri-1,Test-Findit-Wrong&components=Tools%3ETest%3EFindit%3EFlakiness&summary=%5BFindit%5D%20Flake%20Analyzer%20-%20Wrong%20result%20for%20SecurityKeyMessageReaderImplTest.MultipleMessages&comment=Link%20to%20Analysis%3A%20https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVytAELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCJ-Y2hyb21pdW0ubWVtb3J5L0xpbnV4IFRTYW4gVGVzdHMvMjgwMTIvcmVtb3RpbmdfdW5pdHRlc3RzL1UyVmpkWEpwZEhsTFpYbE5aWE56WVdkbFVtVmhaR1Z5U1cxd2JGUmxjM1F1VFhWc2RHbHdiR1ZOWlhOellXZGxjdz09DAsSE01hc3RlckZsYWtlQW5hbHlzaXMYAQw Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).
,
Oct 22
FYI: Test is no longer flaky after revert.
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c8a1dc548deacfd28e32e5f1c54b02ec59dfc911 commit c8a1dc548deacfd28e32e5f1c54b02ec59dfc911 Author: Gabriel Charette <gab@chromium.org> Date: Tue Oct 23 23:10:55 2018 Reland "[MessageLoop] Lock-free ScheduleWork() scheme" This is a reland of f7da13a28c3799819bd39c4d95dc02a805b27e4f (original change in PS1) The original change was missing acquire ordering in DisconnectFromParent(). This is necessary in order for the disconnecting thread to see all memory side-effects previously made by other threads (or some side-effects of message_loop_->ScheduleWork() could racily come in after ~MessageLoop()). Also removed the DCHECK that |operations_state_ == kDisconnectedBit| at the end of DisconnectFromParent() as it was incorrect. A racy BeforeOperation() call can make it 1->3 (and no-op) after DisconnectFromParent() made it 0->1. And lastly, added a scoped allowance to always allow the very fast wait instead of requiring callers to know about this implementation detail of MessageLoop (and reverted changes to //net). Original change's description: > [MessageLoop] Lock-free ScheduleWork() scheme > > The Lock is causing hangs because of priority inversion > mixed with priority boosting (ScheduleWork() tends to > boost the destination thread which may deschedule the > posting thread; if the posting thread is a background > thread this boost-induded-desched-while-holding-lock > can cause a livelock). See https://crbug.com/890978#c10 > for example crashes catching this. > > The Lock was only necessary for startup/shutdown and is > being replaced by a lock-free atomic scheme in this CL. > > MessagePump::ScheduleWork() itself was already thread-safe > (but the Android impl did unnecessarily check a non-atomic bool) > > This adds a WaitableEvent in ~MessageLoop(); hence the requirement > for a wait-allowance in net's EmbeddedTestServer. > > TBR=zhongyi@chromium.org (embedded_test_server.cc side-effects) > > Bug: 890978 , 874237 > Change-Id: I0916e5a99035a935b0a23a770af256f334e78c43 > Reviewed-on: https://chromium-review.googlesource.com/c/1278631 > Commit-Queue: Gabriel Charette <gab@chromium.org> > Reviewed-by: François Doray <fdoray@chromium.org> > Cr-Commit-Position: refs/heads/master@{#601600} Bug: 890978 , 874237, 897925 Change-Id: I17c515f9a3169bbdfc303a4b259f34097e31730d Reviewed-on: https://chromium-review.googlesource.com/c/1297129 Reviewed-by: François Doray <fdoray@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#602166} [modify] https://crrev.com/c8a1dc548deacfd28e32e5f1c54b02ec59dfc911/base/message_loop/message_loop.cc [modify] https://crrev.com/c8a1dc548deacfd28e32e5f1c54b02ec59dfc911/base/message_loop/message_loop.h [modify] https://crrev.com/c8a1dc548deacfd28e32e5f1c54b02ec59dfc911/base/message_loop/message_pump.h [modify] https://crrev.com/c8a1dc548deacfd28e32e5f1c54b02ec59dfc911/base/message_loop/message_pump_android.cc [modify] https://crrev.com/c8a1dc548deacfd28e32e5f1c54b02ec59dfc911/base/threading/thread_restrictions.h
,
Oct 24
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by yigu@chromium.org
, Oct 22Owner: gab@chromium.org
Status: Assigned (was: Untriaged)