Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(394)

Issue 17217: When there are multiple requests for the same resource, it is possible that c... (Closed)

Created:
11 years, 11 months ago by rvargas (doing something else)
Modified:
9 years, 6 months ago
Reviewers:
Peter Kasting, wtc
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

When there are multiple requests for the same resource, it is possible that cancelling a request that is currently a reader may be racing with another reader being completed. In that case, we were not removing the transaction for the cancelled request so all queued requests were blocked forever. R=wtc BUG=4769 TEST=unittest Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=7669

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 7

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+77 lines, -3 lines) Patch
M net/http/http_cache.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M net/http/http_cache_unittest.cc View 1 2 1 chunk +74 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
rvargas (doing something else)
11 years, 11 months ago (2009-01-06 23:30:54 UTC) #1
wtc
LGTM, but I'd like to go over this CL with you in person tomorrow because ...
11 years, 11 months ago (2009-01-07 02:42:30 UTC) #2
rvargas (doing something else)
http://codereview.chromium.org/17217/diff/6/8 File net/http/http_cache.cc (right): http://codereview.chromium.org/17217/diff/6/8#newcode1260 Line 1260: if (entry->will_process_pending_queue && entry->readers.empty()) On 2009/01/07 02:42:30, wtc ...
11 years, 11 months ago (2009-01-07 03:43:08 UTC) #3
Peter Kasting
http://codereview.chromium.org/17217/diff/6/7 File net/http/http_cache_unittest.cc (right): http://codereview.chromium.org/17217/diff/6/7#newcode603 Line 603: for (int i = 0; i < kNumTransactions; ...
11 years, 11 months ago (2009-01-07 18:15:30 UTC) #4
rvargas (doing something else)
On 2009/01/07 18:15:30, pkasting wrote: > http://codereview.chromium.org/17217/diff/6/7 > File net/http/http_cache_unittest.cc (right): > > http://codereview.chromium.org/17217/diff/6/7#newcode603 > ...
11 years, 11 months ago (2009-01-07 18:29:15 UTC) #5
wtc
11 years, 11 months ago (2009-01-07 18:44:45 UTC) #6
Ricardo and I just went over the CL.  Now I understand
the CL much better and I am happy with his response to
my review comments.  This code is really subtle.

Powered by Google App Engine
This is Rietveld 408576698