Test was examining a variable without first acquiring a required Lock....

Test was examining a variable without first acquiring a required Lock.

Also changed Lock::AssertAcquired to be const function (it already was const, in practice, just not declared that way).

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=12157

[ralphl, 2009-03-19 18:48:46]

When shutting down threads, there was a variable that needed to be examined
under a lock.  The test code made sure that the lock SHOULD have been taken, and
it had not.  The new code actually takes the lock.

I also added calls to the new Lock::AssertAcquired() method in a few spots.

The last part of the change is that I changed AssertAcquired() to be const,
which I should have been all along (no code changed, just the method
declaration)

[jar (doing other things), 2009-03-19 19:23:27]

See comments below.

Thanks for spotting this!!!

http://codereview.chromium.org/42402/diff/1/2
File base/condition_variable_unittest.cc (right):

http://codereview.chromium.org/42402/diff/1/2#newcode188
Line 188: // it appears that the falkyness is gone, but I'll leave that to you
to verify.
falkyness->flakyness

http://codereview.chromium.org/42402/diff/1/2#newcode539
Line 539: // lock already acquired.
Nested locks are not supported.  Either correct the calling context (to have the
lock in all needed places) or rename this new method to something different like
ThreadSafeShutdownTaskCount().

[ralphl, 2009-03-19 20:17:54]

Incorporated jar's comments.  Changed method from simple getter to a real
function.  Fixed comment typos.

http://codereview.chromium.org/42402/diff/1/2
File base/condition_variable_unittest.cc (right):

http://codereview.chromium.org/42402/diff/1/2#newcode188
Line 188: // it appears that the falkyness is gone, but I'll leave that to you
to verify.
On 2009/03/19 19:23:27, jar wrote:
> falkyness->flakyness

Done.

http://codereview.chromium.org/42402/diff/1/2#newcode539
Line 539: // lock already acquired.
On 2009/03/19 19:23:27, jar wrote:
> Nested locks are not supported.  Either correct the calling context (to have
the
> lock in all needed places) or rename this new method to something different
like
> ThreadSafeShutdownTaskCount().

Changed name and semantics.  Returns true if matches number passed in.  See if
you like new method name / implementation.

[jar (doing other things), 2009-03-19 20:44:11]

LGTM

[cpu_(ooo_6.6-7.5), 2009-03-20 00:11:44]

RSLGTM