New issue
Advanced search Search tips

Issue 904978 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 16
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 871690



Sign in to add a comment

Race condition between Secondary Account removals from AccountTrackerService and ARC

Project Member Reported by sinhak@chromium.org, Nov 13

Issue description

Context:
- Chrome OS Account Manager uses Gaia ID as the account identifier for Gaia accounts.
- ARC uses Email ID as the account identifier for Gaia accounts.
- To translate between these IDs, ArcAuthService (transitively) relies on AccountTrackerService e.g. https://cs.chromium.org/chromium/src/chrome/browser/chromeos/arc/auth/arc_auth_service.cc?l=482&rcl=147873d1ebb54309fd019b84d42d29da746b16d6

Problem:
Account removals from Chrome OS Account Manager trigger a series of removals for that account, including removals inside AccountTrackerService and ArcAuthService.

ArcAuthService depends on AccountTrackerService to translate Gaia IDs to Email IDs to be able to propagate this update to ARC.

However, a race condition exists between the 2 aforementioned removals which sometimes triggers a DCHECK (https://cs.chromium.org/chromium/src/chrome/browser/chromeos/arc/auth/arc_auth_service.cc?l=484&rcl=147873d1ebb54309fd019b84d42d29da746b16d6) i.e. an account may get removed from AccountTrackerService *before* ArcAuthService had a chance to get the account's email ID.
 
Labels: -Restrict-View-Google
Blocking: 871690
Description: Show this description
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 16

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

commit eaf5778e5ca3aea9b5724e6d08d43780e2731482
Author: Kush Sinha <sinhak@chromium.org>
Date: Fri Nov 16 18:08:06 2018

Fix race condition in ArcAuthService

Please first check the attached bug for context.

The fix is to listen on AccountTrackerService and not AccountManager
for account removals.

We cannot listen on AccountTrackerService for account updates because
AccountTrackerService does not notify its observers when an account's
LST changes.

Bug:  904978 
Test: browser_tests --gtest_filter="ArcAuthServiceTest.AccountRemovalsArePropagated"
Change-Id: I75aeda8dd280734e58e509f5a34a4b1e27b09498
Reviewed-on: https://chromium-review.googlesource.com/c/1334368
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608853}
[modify] https://crrev.com/eaf5778e5ca3aea9b5724e6d08d43780e2731482/chrome/browser/chromeos/arc/auth/arc_auth_service.cc
[modify] https://crrev.com/eaf5778e5ca3aea9b5724e6d08d43780e2731482/chrome/browser/chromeos/arc/auth/arc_auth_service.h
[modify] https://crrev.com/eaf5778e5ca3aea9b5724e6d08d43780e2731482/chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc

Status: Fixed (was: Started)

Sign in to add a comment