Make sure that the LoadLog does not get freed on the worker thread during req...

Make sure that the LoadLog does not get freed on the worker thread during request cancellation.

BUG=22272
TEST=must pass existing tests when running with TSAN.


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

[eroman, 2009-09-18 19:39:16]

Either of timurrr or wtc please review this.

(I will consider an LGTM from either as sufficient to commit, but will certainly
address any other feedback as followups)

[the_wrong_timurrrr, 2009-09-18 20:06:25]

On 2009/09/18 19:39:16, eroman wrote:
> Either of timurrr or wtc please review this.
> 
> (I will consider an LGTM from either as sufficient to commit, but will
certainly
> address any other feedback as followups)
I'm not very familiar with the code, so I guess my LGTM won't be sufficient.
However, I'll try your change under ThreadSanitizer soon.

[the_wrong_timurrrr, 2009-09-18 20:23:16]

On 2009/09/18 20:06:25, the_wrong_timurrrr wrote:
> On 2009/09/18 19:39:16, eroman wrote:
> > Either of timurrr or wtc please review this.
> > 
> > (I will consider an LGTM from either as sufficient to commit, but will
> certainly
> > address any other feedback as followups)
> I'm not very familiar with the code, so I guess my LGTM won't be sufficient.
> However, I'll try your change under ThreadSanitizer soon.
Sorry, can't test it today since gclient gone mad and it's past midnight in
Moscow...

[the_wrong_timurrrr, 2009-09-18 20:38:30]

On 2009/09/18 20:23:16, the_wrong_timurrrr wrote:
> On 2009/09/18 20:06:25, the_wrong_timurrrr wrote:
> > On 2009/09/18 19:39:16, eroman wrote:
> > > Either of timurrr or wtc please review this.
> > > 
> > > (I will consider an LGTM from either as sufficient to commit, but will
> > certainly
> > > address any other feedback as followups)
> > I'm not very familiar with the code, so I guess my LGTM won't be sufficient.
> > However, I'll try your change under ThreadSanitizer soon.
> Sorry, can't test it today since gclient gone mad and it's past midnight in
> Moscow...
Tested your patch in "hybrid mode" (much more aggressive than the on the bot),
LGTM++

[wtc, 2009-09-21 18:14:12]

LGTM.  The comments you added are a little ambiguous.

http://codereview.chromium.org/214025/diff/1/2
File net/base/host_resolver_impl.cc (right):

http://codereview.chromium.org/214025/diff/1/2#newcode76
Line 76: // Clear the LoadLog to make sure it won't be released later on the
Do you mean clearing the load log (i.e., emptying the load
log), or releasing our reference to the load log?

http://codereview.chromium.org/214025/diff/1/2#newcode399
Line 399: // Hold a reference to the request's load log as we are about to clear
it.
Nit: it's not clear what "it" refers to here.  Do you mean
clearing the request's reference to the load log, or clearing
the load log itself?  I think you mean the former (clearing
the req->load_log_ field).