OSCryptConcurrencyTest.ConcurrentInitialization flaky/failing on Mac Debug |
|||
Issue descriptionOSCryptConcurrencyTest.ConcurrentInitialization is failing on Mac debug local build, as reported in http://crbug.com/655950#c4 . The stacktrace attached there shows lock_impl_posix.cc:53 as the failing DCHECK: frame #3: 0x0000000111d6ad2e libbase.dylib`base::internal::LockImpl::~LockImpl(this=0x00000001098e0d50) + 238 at lock_impl_posix.cc:53 50 51 LockImpl::~LockImpl() { 52 int rv = pthread_mutex_destroy(&native_handle_); -> 53 DCHECK_EQ(rv, 0) << ". " << strerror(rv); 54 } 55 56 bool LockImpl::Try() {
,
Oct 18 2016
,
Oct 18 2016
First of all, did you run the test multiple times to establish whether it's failing or just flaky? I had a quick look at the CL, and I don't think changes in autofill should be affecting this test. If the test is not flaky, then I would suspect some concurrency side-effect coming from somewhere (hooray!)
,
Oct 18 2016
I just ran a couple of tests, the error appeared in 5 out of 10 trials. My wild guess would be a race condition, could that be the case here? Success rate increases to 8/10 when bumping up the number of threads in the test to 4 instead of 2.
,
Oct 18 2016
The calls that the test makes are (supposed to be) thread-safe. Since the test is just flaky, you don't have to block your work on this. I'd run the test on a clean branch too, just to make sure the flakiness isn't accentuated by the CL. The test isn't flaky on Linux, therefore I can't investigate myself until I get my hands on a MacOS.
,
Oct 18 2016
One more note, if there is a race condition, TSan should be able to detect it. https://www.chromium.org/developers/testing/threadsanitizer-tsan-v2
,
Nov 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e9fc2e0c783274f4edd3adb175edf39d2ea8327b commit e9fc2e0c783274f4edd3adb175edf39d2ea8327b Author: jdoerrie <jdoerrie@chromium.org> Date: Thu Nov 03 12:48:58 2016 Make GetEncryptionKey thread-safe The current implementation of |GetEncryptionKey()| is thread-unsafe, resulting in a flaky unit test. This change fixes this issue by making the lock a |LazyInstance| instead of static. BUG= 656926 R=cfroussios@chromium.org Review-Url: https://codereview.chromium.org/2472743002 Cr-Commit-Position: refs/heads/master@{#429547} [modify] https://crrev.com/e9fc2e0c783274f4edd3adb175edf39d2ea8327b/components/os_crypt/os_crypt_mac.mm
,
Nov 3 2016
|
|||
►
Sign in to add a comment |
|||
Comment 1 by vabr@chromium.org
, Oct 18 2016