HttpCache::Transaction::StopCaching should give up the cache lock when it can. |
|||
Issue descriptionHttpCache::Transaction::StopCaching() does not give up the cache lock. As per the code comment: "We really don't know where we are now. Hopefully there is no operation in progress, but nothing really prevents this method to be called after we returned ERR_IO_PENDING. We cannot attempt to truncate the entry at this point because we need the state machine for that (and even if we are really free, that would be an asynchronous operation). In other words, keep the entry how it is (it will be marked as truncated at destruction), and let the next piece of code that executes know that we are now reading directly from the net. TODO(mmenke): This doesn't release the lock on the cache entry, so a future request for the resource will be blocked on this one. Fix this." Basically it should check if the state is STATE_NONE, which means that its not waiting for any asynchronous call to complete, then invoke cache_->DoneWithEntry.
,
Jun 2 2017
Another factor to consider is that currently StopCaching() is a synchronous call while truncation is asynchronous.
,
Jun 5 2017
https://codereview.chromium.org/2721933002 fixes the StopCaching logic such that this entry is doomed and any queued transactions are restarted to create a new entry. Though this will lead to the original entry not being used but currently this does not impact any scenario since StopCaching() is immediately invoked after receiving headers. Also, this allows no future or simultaneous transactions to be blocked on the first transaction if its not caching.
,
Jun 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8061c420676998bda77caa74581ea8061860f438 commit 8061c420676998bda77caa74581ea8061860f438 Author: shivanisha <shivanisha@chromium.org> Date: Tue Jun 13 23:35:52 2017 This CL is a precursor to allowing shared writing to fix cache lock. This CL allows transactions to continue to their validation phase even when another transaction is the active reader/writer. After the validation phase, if its a match the transaction might wait till the response is written to the cache by the active writer. If its not a match the transaction will doom the entry and go to the network. In a subsequent CL, the not matching case will create a new entry as well. BUG= 472740 , 715913 , 715974 , 715920 , 715911 , 713348 Review-Url: https://codereview.chromium.org/2721933002 Cr-Original-Commit-Position: refs/heads/master@{#467426} Committed: https://chromium.googlesource.com/chromium/src/+/1e2e347f957ef889aaee527bb757849f76e8a808 Review-Url: https://codereview.chromium.org/2721933002 Cr-Commit-Position: refs/heads/master@{#479204} [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_cache.cc [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_cache.h [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_cache_transaction.cc [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_cache_transaction.h [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_cache_unittest.cc [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_transaction.h [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_transaction_test_util.cc [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_transaction_test_util.h [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/mock_http_cache.cc [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/mock_http_cache.h [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/url_request/url_request_http_job_unittest.cc [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/url_request/url_request_quic_unittest.cc [modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/url_request/url_request_unittest.cc
,
Jul 17 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by shivanisha@chromium.org
, Apr 19 2017