New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 732406 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Signin access the command line from the IO thread

Project Member Reported by droger@chromium.org, Jun 12 2017

Issue description

As per the CommandLine documentation, it is not thread safe.
However, signin code accesses it on the IO thread, for account consistency switches.

See some discussion in https://codereview.chromium.org/2925083002
 

Comment 1 by droger@chromium.org, Jun 13 2017

Cc: brettw@chromium.org
I added a thread checker in CommandLine::GetForCurrentProcess() and I could see many calls from other threads.

Example callers:

IOThread::Init()
content::IsBrowserSideNavigationEnabled() (called from ResourceDispatcherHostImpl)
extensions::ChromeExtensionsClient::GetWebstoreUpdateURL()
content::ForeignFetchRequestHandler::IsForeignFetchEnabled()

This chromium-dev@ thread seems to imply that it is actually safe to do:
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/VwDNv8FqVx0/0Le3mznZj_AJ

brettw: do you know if that's OK to read CommandLine switches from background threads?

Project Member

Comment 2 by sheriffbot@chromium.org, Jul 14 2017

Status: Available (was: Assigned)
--Chrome Identity automated triaging--

This bug is Assigned and has gone one month without any activity, so it is being moved to Available to indicate that it is not actively being worked on. If you are working on this bug, please mark yourself as the owner and move back to Assigned. Please see https://goo.gl/78kbny for more details. Please remove the Services>SignIn or UI>Browser>Profiles components if this bug isn't related to Chrome Identity.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 3 by msarda@chromium.org, Jul 17 2017

Status: Assigned (was: Available)
Keeping this assigned. 
David: Do you have any news about this? Is is a problem to use the CommandLine from the IO thread?

Comment 4 by droger@chromium.org, Jul 17 2017

Nothing new here.

From what I understand the class is not thread safe, but it works in Chrome because the command line is never changed after startup.

It is somewhat hacky though, because it is not enforced in any way, and someone could introduce subtle a crash easily, by simply appending a switch from what seems to be the right thread.

Not sure what to do though. The CommandLine class should maybe be changed to become "read-only" after startup?


Comment 5 by msarda@chromium.org, Jul 17 2017

I think we should close this bug as WontFix or assign it to brett@ if we expect an answer from him for comment #1. WDYT?
Project Member

Comment 6 by sheriffbot@chromium.org, Aug 16 2017

Status: Available (was: Assigned)
--Chrome Identity automated triaging--

This bug is Assigned and has gone one month without any activity, so it is being moved to Available to indicate that it is not actively being worked on. If you are working on this bug, please mark yourself as the owner and move back to Assigned. Please see https://goo.gl/78kbny for more details. Please remove the Services>SignIn or UI>Browser>Profiles components if this bug isn't related to Chrome Identity.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 7 by droger@chromium.org, Oct 24 2017

Status: WontFix (was: Available)

Sign in to add a comment