SandboxTest.SpawnSandboxTarget (chrome_cleaner_unittests) is flaky |
|||||||
Issue descriptionFiled by sheriff-o-matic@appspot.gserviceaccount.com on behalf of mstensho@chromium.org chrome_cleaner_unittests failing on chromium.win/Win10 Tests x64 (dbg) Builders failed on: - Win10 Tests x64 (dbg): https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win10%20Tests%20x64%20%28dbg%29 Flaky at least since: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win10%20Tests%20x64%20%28dbg%29/2373 (2018-08-08)
,
Aug 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/802385a55780c99158d6ad37d2f64a2b2772aeb2 commit 802385a55780c99158d6ad37d2f64a2b2772aeb2 Author: Morten Stenshorne <mstensho@chromium.org> Date: Wed Aug 15 09:59:04 2018 Disable SandboxTest.SpawnSandboxTarget from chrome_cleaner_unittests It's flaky. TBR=joenotcharles@chromium.org Bug: 874387 Change-Id: I3056f49cb1879a6fbacd195f94dfe6992cfd6710 Reviewed-on: https://chromium-review.googlesource.com/1175123 Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/master@{#583209} [modify] https://crrev.com/802385a55780c99158d6ad37d2f64a2b2772aeb2/chrome/chrome_cleaner/ipc/sandbox_unittest.cc
,
Aug 15
,
Aug 15
I think this code in sandbox.cc has a race condition:
// Wait for the sandboxed process to signal it is ready, checking every now
// and then to see if the process crashed before it could complete it's setup.
int exit_code = -1;
while (!init_done_event->TimedWait(base::TimeDelta::FromSeconds(1))) {
if (process_handle.WaitForExitWithTimeout(base::TimeDelta(), &exit_code)) {
LOG(ERROR)
<< "Sandboxed process exited before signaling it was initialized, "
"exit code: "
<< exit_code;
return RESULT_CODE_FAILED_TO_START_SANDBOX_PROCESS;
}
}
In tests the sandboxed process signals the event and then exits almost immediately. If the parent returns from TimedWait, then the sandbox process signals the event and exits before the call to process_handle.WaitForExitWithTimeout, the parent process will miss the signal.
Solution is to add both the event and process handles to WaitForMultipleObjects and then check which one was signaled. (See http://shortn/_I3Ua6xmMXA for a test that does this in the internal repo.)
,
Aug 15
,
Aug 15
,
Aug 15
,
Aug 16
,
Aug 17
,
Aug 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/84ed7b5aa12e9b73b9db07bc0d76420eb30b8820 commit 84ed7b5aa12e9b73b9db07bc0d76420eb30b8820 Author: Joe Mason <joenotcharles@chromium.org> Date: Wed Aug 22 14:14:19 2018 Fix race condition in StartSandboxTarget If the sandbox target process is very short-lived (as in some multiprocess tests), and also takes long enough to spawn that the first TimedWait on the init done notifier times out, then there's a race between the target process exiting and the parent process calling TimedWait again. Instead of polling in a loop, use WaitForMultipleObjects to wait for both the init done notifier and process exit. TBR=csharp TBR_REASON=Already reviewed in internal repo. Bug: 874387 Change-Id: I86af661bbe43f3fc044fb5afd6bcbd9f9d7b297c Reviewed-on: https://chromium-review.googlesource.com/1184214 Reviewed-by: Joe Mason <joenotcharles@chromium.org> Reviewed-by: Chris Sharp <csharp@chromium.org> Commit-Queue: Chris Sharp <csharp@chromium.org> Cr-Commit-Position: refs/heads/master@{#585006} [modify] https://crrev.com/84ed7b5aa12e9b73b9db07bc0d76420eb30b8820/chrome/chrome_cleaner/ipc/sandbox.cc
,
Aug 22
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by mstensho@chromium.org
, Aug 15Status: Assigned (was: Available)