New issue
Advanced search Search tips

Issue 782487 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

chromeos DCHECKS on ProcessSingleton rendezvous (ProcessSingletonTest.StartupRaceCondition disabled on chromeos)

Project Member Reported by mattm@chromium.org, Nov 8 2017

Issue description

ProcessSingletonTest.StartupRaceCondition has been disabled for a long time ( issue 513534 ). I'm updating and re-enabling the test, but chromeos fails as various things in ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun have DCHECKs that they were initialized before being shutdown.  (eg, NoteTakingHelper::Shutdown, ScreenLocker::ShutDownClass, and probably more.) When a ProcessSingleton rendezvous happens, the browser process starts shutting down before initializing those objects, but the shutdown code still gets called.

* I don't know if it's a bug that ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun is getting called even though the ProcessSingleton said not to start the browser?
* Or is it a bug that the NoteTakingHelper::Shutdown is in PostMainMessageLoopRun but the NoteTakingHelper::Initialize is in PreProfileInit (not PreMainMessageLoopRun)?
* Or should they allow shutdown getting called even if they weren't initialized?
* Does chromeos even need ProcessSingleton?

I don't really know how this startup/shutdown code is intended to work. Sorry for CC spam, there doesn't seem to be a component that covers this so just cc-ing some people from blame that might have some idea how this should work.
 

Comment 1 by derat@chromium.org, Nov 8 2017

Cc: xiy...@chromium.org
I don't know much about startup or shutdown, but is this related to issue 702403 ("DCHECK in NoteTakingHelper::Shutdown in ChromeMainTest.ReuseBrowserInstanceWhenOpeningFile")?
ProcessSingleton is not useful on chromeos today but it might in the future.

Think the root cause is similar to issue 702403. Maybe we should move the init code of those have shutdown code in PostMainMessageLoopRun to PreMainMessageLoopRun to make them symmetric.

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6808672b64d5e001d108920472640488a3b5d4d1

commit 6808672b64d5e001d108920472640488a3b5d4d1
Author: Matt Mueller <mattm@chromium.org>
Date: Thu Nov 09 19:24:52 2017

Update and re-enable ProcessSingletonTest.StartupRaceCondition (except for ChromeOS)

Bug:  513534 , 782487
Change-Id: I56befe3da12959689762b0be60df0c186d5eaa14
Reviewed-on: https://chromium-review.googlesource.com/619659
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515237}
[modify] https://crrev.com/6808672b64d5e001d108920472640488a3b5d4d1/base/process/kill.cc
[modify] https://crrev.com/6808672b64d5e001d108920472640488a3b5d4d1/chrome/browser/process_singleton_browsertest.cc

ProcessSingletonTest.StartupRaceCondition has been flaky on Linux and Win bots

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=ProcessSingletonTest.StartupRaceCondition
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4d92a15f70bd435fbc903389bc3e92e0d8a7ffc

commit f4d92a15f70bd435fbc903389bc3e92e0d8a7ffc
Author: Takeshi Yoshino <tyoshino@chromium.org>
Date: Mon Nov 20 08:40:40 2017

Disable a flaky test ProcessSingletonTest.StartupRaceCondition also on Linux and Win

Bug: 782487
Change-Id: Ied52b6322d1e66f11ae589256680b48706a62b67
Tbr: jochen@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/778601
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Commit-Queue: Takeshi Yoshino <tyoshino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517783}
[modify] https://crrev.com/f4d92a15f70bd435fbc903389bc3e92e0d8a7ffc/chrome/browser/process_singleton_browsertest.cc

Sign in to add a comment