Remove unnecessary lock that causes reentry.

Remove unnecessary lock that causes reentry.

The StatisticsRecorder is acquiring a lock before
calling Import but the import calls back to the SR
which again tries to acquire the lock... and crashes.

The lock isn't necessary with the new persistent
iterator (which is lock-free) so move the Import
call outside of the locking.

BUG=601457
Committed: https://crrev.com/2f153f83f1d6fcfb4b0b8dae5a6b12dbc6544d6b
Cr-Commit-Position: refs/heads/master@{#386102}

[bcwhite, 2016-04-08 06:14:33]

bcwhite@chromium.org changed reviewers:
+ asvitkine@chromium.org

[bcwhite, 2016-04-08 13:24:27]

On 2016/04/08 06:14:34, bcwhite wrote:

I tried to write a test for this but cannot until the test-order fix is in:
https://codereview.chromium.org/1779503002/

[bcwhite, 2016-04-08 14:44:47]

Description was changed from

==========
Remove unnecessary lock that causes reentry.

The StatisticsRecorder is acquiring a lock before
calling Import but the import calls back to the SR
which again tries to acquire the lock... and crashes.

The lock isn't necessary with the new persistent
iterator (which is lock-free) so move the Import
call outside of the locking.

BUG=546019
==========

to

==========
Remove unnecessary lock that causes reentry.

The StatisticsRecorder is acquiring a lock before
calling Import but the import calls back to the SR
which again tries to acquire the lock... and crashes.

The lock isn't necessary with the new persistent
iterator (which is lock-free) so move the Import
call outside of the locking.

BUG=601457
==========

[Alexei Svitkine (slow), 2016-04-08 14:51:49]

lgtm

I agree that this should have test, please work on that after landing this.

I'll review the other CL that fixes the testing story now.

https://codereview.chromium.org/1872713002/diff/1/base/metrics/statistics_rec...
File base/metrics/statistics_recorder.cc (right):

https://codereview.chromium.org/1872713002/diff/1/base/metrics/statistics_rec...
base/metrics/statistics_recorder.cc:297: base::AutoLock auto_lock(*lock_);
Add a comment for why this is done here and not at the top.

[bcwhite, 2016-04-08 15:06:08]

Already started the test based on the order-fix CL.

https://codereview.chromium.org/1872713002/diff/1/base/metrics/statistics_rec...
File base/metrics/statistics_recorder.cc (right):

https://codereview.chromium.org/1872713002/diff/1/base/metrics/statistics_rec...
base/metrics/statistics_recorder.cc:297: base::AutoLock auto_lock(*lock_);
On 2016/04/08 14:51:48, Alexei Svitkine wrote:
> Add a comment for why this is done here and not at the top.

Done.

[bcwhite, 2016-04-08 15:06:31]

The CQ bit was checked by bcwhite@chromium.org

[bcwhite, 2016-04-08 15:06:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1872713002/#ps20001
(title: "added comment as to why Import is done before lock")

[commit-bot: I haz the power, 2016-04-08 15:06:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1872713002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1872713002/20001

[commit-bot: I haz the power, 2016-04-08 16:15:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-08 16:15:33]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[bcwhite, 2016-04-08 16:51:42]

The CQ bit was checked by bcwhite@chromium.org

[commit-bot: I haz the power, 2016-04-08 16:52:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1872713002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1872713002/20001

[commit-bot: I haz the power, 2016-04-08 16:58:10]

Description was changed from

==========
Remove unnecessary lock that causes reentry.

The StatisticsRecorder is acquiring a lock before
calling Import but the import calls back to the SR
which again tries to acquire the lock... and crashes.

The lock isn't necessary with the new persistent
iterator (which is lock-free) so move the Import
call outside of the locking.

BUG=601457
==========

to

==========
Remove unnecessary lock that causes reentry.

The StatisticsRecorder is acquiring a lock before
calling Import but the import calls back to the SR
which again tries to acquire the lock... and crashes.

The lock isn't necessary with the new persistent
iterator (which is lock-free) so move the Import
call outside of the locking.

BUG=601457
==========

[commit-bot: I haz the power, 2016-04-08 16:58:11]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-08 16:59:42]

Description was changed from

==========
Remove unnecessary lock that causes reentry.

The StatisticsRecorder is acquiring a lock before
calling Import but the import calls back to the SR
which again tries to acquire the lock... and crashes.

The lock isn't necessary with the new persistent
iterator (which is lock-free) so move the Import
call outside of the locking.

BUG=601457
==========

to

==========
Remove unnecessary lock that causes reentry.

The StatisticsRecorder is acquiring a lock before
calling Import but the import calls back to the SR
which again tries to acquire the lock... and crashes.

The lock isn't necessary with the new persistent
iterator (which is lock-free) so move the Import
call outside of the locking.

BUG=601457

Committed: https://crrev.com/2f153f83f1d6fcfb4b0b8dae5a6b12dbc6544d6b
Cr-Commit-Position: refs/heads/master@{#386102}
==========

[commit-bot: I haz the power, 2016-04-08 16:59:43]

Patchset 2 (id:??) landed as
https://crrev.com/2f153f83f1d6fcfb4b0b8dae5a6b12dbc6544d6b
Cr-Commit-Position: refs/heads/master@{#386102}