New issue
Advanced search Search tips

Issue 687991 link

Starred by 1 user
Status: Duplicate
Owner:
emaxx@chromium.org
Closed: Mar 2017
Cc:
atwilson@chromium.org
tnagel@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Investigate the correctness of CancelableClosure usage in CloudPolicyRefreshScheduler

Project Member Reported by emaxx@chromium.org, Feb 2 2017 
The CloudPolicyRefreshScheduler class contains the refresh_callback_ member of type base::CancelableClosure. This member is assigned with a bound PerformRefresh() call.
The problematic thing here is that PerformRefresh() itself calls Cancel() on refresh_callback_. In other words, the callback is canceling itself.

We should investigate whether such construction is correct. It's quite likely that this is an incorrect usage of CancelableClosure, and we didn't observe crashes in this places only because an Unretained(this) is all that is passed to the callback.

Comment 1 by emaxx@chromium.org, Feb 20 2017 

Looks like, to start with, the thread safety requirements documented with CancelableCallback are not respected by some pieces in the code (although probably not in the policy related code).

I've created a separate bug to add the corresponding DCHECK's into CancelableCallback and fix the code that violates them: issue 694268.

Comment 2 by emaxx@chromium.org, Mar 2 2017

Mergedinto: 697738
Status: Duplicate (was: Assigned)
So this usage (doing Cancel() from the callback) was technically incorrect. It was relatively safe because only Unretained(this) was bound to it.

But instead of fixing this here (and in 10+ other places in the Chrome code base), I'll attempt to change CancelableCallback to support this: see bug 697738.
►

Sign in to add a comment