ProcessUtilTest.DelayedTermination & ImmediateTermination flake on Fuchsia |
||||
Issue descriptionhttps://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FFuchsia%2F7730%2F%2B%2Frecipes%2Fsteps%2Fbase_unittests%2F0%2Fstdout [00103.700] 02675.02703> [ RUN ] ProcessUtilTest.DelayedTermination [00103.700] 02675.02703> ../../base/process/process_util_unittest.cc:898: Failure [00103.700] 02675.02703> Value of: IsProcessDead(spawn_child.process.Handle()) [00103.700] 02675.02703> Actual: false [00103.700] 02675.02703> Expected: true The test waits for 2s, then calls mx_task_kill(), then waits for 5s for that, then checks to make sure that it's dead. I believe this does mx_object_wait_one(..., MX_TASK_TERMINATED, ...) twice, once on the 5s wait, and then a second time on the "make sure it's dead". Maybe that's not correct?
,
Aug 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/19c8abf569cd28484140224e8e15a344e7e4c5b3 commit 19c8abf569cd28484140224e8e15a344e7e4c5b3 Author: Scott Graham <scottmg@chromium.org> Date: Tue Aug 01 00:41:56 2017 Some fuchsia DLOG/DCHECK to LOG/CHECK to diagnose crbug.com/750756 Bug: 750756 , 738275 Change-Id: Iccc8d0d5a279063d9ca2bb0eecb4ff95920c81d8 Reviewed-on: https://chromium-review.googlesource.com/594931 Commit-Queue: Scott Graham <scottmg@chromium.org> Reviewed-by: Wez <wez@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#490827} [modify] https://crrev.com/19c8abf569cd28484140224e8e15a344e7e4c5b3/base/process/process_fuchsia.cc
,
Aug 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9759fff7fc48f5d70e0555d51a54540536cf9cc7 commit 9759fff7fc48f5d70e0555d51a54540536cf9cc7 Author: Scott Graham <scottmg@chromium.org> Date: Wed Aug 02 01:53:03 2017 fuchsia: Filter out some known-flake tests in base_unittests Bug: 738275, 735701 , 750370 , 750756 Change-Id: Ibaa143616ce60ae6d94854f9cadc8daf2d84a37c Reviewed-on: https://chromium-review.googlesource.com/596914 Reviewed-by: Wez <wez@chromium.org> Commit-Queue: Scott Graham <scottmg@chromium.org> Cr-Commit-Position: refs/heads/master@{#491200} [modify] https://crrev.com/9759fff7fc48f5d70e0555d51a54540536cf9cc7/testing/buildbot/filters/fuchsia.base_unittests.filter
,
Jan 27 2018
We're seeing ProcessUtilTest.ImmediateTermination also flake, under Fuchsia/Debug. EnsureProcessTermination() has two distinct implementation behaviours: - Windows+Linux: If the process isn't already dead then a background thread waits 2s, then checks again before kill()ing it, and cleanup up the handle. - Mac+Fuchsia: If the process isn't already dead then block for up to 2s, then check again, kill & clean up. Under Fuchsia, our implementation invokes process.Terminate() after 2s, but without setting |wait| to true; since the underlying task kill operation is necessarily asynchronous, this can result in us returning to the test body and having EXPECT_TRUE(IsProcessDead()) fail, before the process is actually dead. EnsureProcessTermination is only used in four places, three of which are Linux or Mac specific; we should either restrict this API to being provided & used on those platforms, or fix the Mac & Fuchsia to be more consistent.
,
Jan 27 2018
,
Jan 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/996812e1c9b72d29bac5cf688d9f89a7b9ffa535 commit 996812e1c9b72d29bac5cf688d9f89a7b9ffa535 Author: Wez <wez@chromium.org> Date: Mon Jan 29 20:00:49 2018 Workaround for EnsureProcessTerminated to address ProcessUtilTest failures. Bug: 750756 , 806451 Change-Id: I60ab7ca8a99bd1845492d7486e811f68105116f2 Reviewed-on: https://chromium-review.googlesource.com/890109 Reviewed-by: Scott Graham <scottmg@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#532558} [modify] https://crrev.com/996812e1c9b72d29bac5cf688d9f89a7b9ffa535/base/process/kill_fuchsia.cc
,
Apr 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c18a57c2b9c89230c5f67d2a06ec527499db0cfd commit c18a57c2b9c89230c5f67d2a06ec527499db0cfd Author: Wez <wez@chromium.org> Date: Mon Apr 02 20:20:14 2018 Simplify EnsureProcessTerminated() implementations. EnsureProcessTerminated() is used by a parent process to ensure that a child process it expects should exit "soon" actually does so. The child process is asynchronously monitored, and forcibly terminated if it fails to exit in a timely manner. Under POSIX platforms the child process Id is also reaped with waitpid(), to release its exited "zombie" process structure. We had ended up with separate implementations for each platform, each with different properties. This CL makes the following changes: - Reimplements POSIX EnsureProcessTerminated using base::Process. - Provides a common implementation for platforms on which there is no need to waitpid() to cleanup zombie processes. The Mac specialization will be replaced with an asynchronous implementation in a subsequent patch. Bug: 806451, 750756 Change-Id: If251dc13e7ad0a0cffb4f1921897a89305d6cb21 Reviewed-on: https://chromium-review.googlesource.com/920799 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Bo <boliu@chromium.org> Cr-Commit-Position: refs/heads/master@{#547508} [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/base/process/kill.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/base/process/kill.h [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/base/process/kill_fuchsia.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/base/process/kill_posix.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/base/process/kill_win.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/base/process/process_posix.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/base/process/process_util_unittest.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/chrome/browser/extensions/api/messaging/native_message_process_host.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/chrome/browser/platform_util_linux.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/chrome/browser/printing/printer_manager_dialog_linux.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/chrome/browser/ui/webui/settings_utils_linux.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/content/browser/child_process_launcher_helper_linux.cc [modify] https://crrev.com/c18a57c2b9c89230c5f67d2a06ec527499db0cfd/content/browser/zygote_host/zygote_host_impl_linux.cc
,
Apr 2 2018
,
Jun 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a6c9bb702fb6316f70db42b0d3f60cbb9e2d8f7c commit a6c9bb702fb6316f70db42b0d3f60cbb9e2d8f7c Author: Wez <wez@chromium.org> Date: Thu Jun 21 00:50:34 2018 Clean up base::Process logging under Fuchsia. Bug: 750756 Change-Id: I6fac894ec5aa6ba2cb7aab58b8b1aa03769adadf Reviewed-on: https://chromium-review.googlesource.com/1108502 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Scott Graham <scottmg@chromium.org> Cr-Commit-Position: refs/heads/master@{#569111} [modify] https://crrev.com/a6c9bb702fb6316f70db42b0d3f60cbb9e2d8f7c/base/process/process_fuchsia.cc |
||||
►
Sign in to add a comment |
||||
Comment 1 by scottmg@chromium.org
, Jul 31 2017