From ef5adf65e8dcd6202bbb4d17f91dd1774ff14446 Mon Sep 17 00:00:00 2001
From: Maks Orlovich <[email protected]>
Date: Thu, 15 Oct 2026 18:43:59 -0700
Subject: [PATCH] HttpCache: prevent lock timeout overtaking explicit restart

...by cancelling it when queuing the restart. This can happen if our
thread is super-overloaded (which is worrisome)

Bug: 569674221
Change-Id: Iaa3f12540534ca58c2446102d6b79e3566a557c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8578804
Commit-Queue: Maks Orlovich <[email protected]>
Reviewed-by: Kenichi Ishibashi <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1720263}
---
diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index 13ef0046..fb45d1f 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -1437,6 +1437,13 @@
   // for the transaction to not be found in this entry.
   for (HttpCache::Transaction* transaction : entry->add_to_entry_queue()) {
     transaction->ResetCachePendingState();
+
+    // Cancel the timeout timer. Under extreme load that can end up firing
+    // before the task we post below, and our tasks expects us to be removed
+    // from the entry queue, which we might no longer be the case with the
+    // reversal.
+    transaction->CancelEntryLockTimeout();
+
     TaskRunner(transaction->priority())
         ->PostTask(FROM_HERE, base::BindOnce(transaction->cache_io_callback(),
                                              ERR_CACHE_RACE));
diff --git a/net/http/http_cache_transaction.h b/net/http/http_cache_transaction.h
index b4375716..bfd85145 100644
--- a/net/http/http_cache_transaction.h
+++ b/net/http/http_cache_transaction.h
@@ -182,6 +182,10 @@
   // added to an entry and the entry.
   void ResetCachePendingState() { cache_pending_ = false; }
 
+  void CancelEntryLockTimeout() {
+    entry_lock_waiting_since_ = base::TimeTicks();
+  }
+
   RequestPriority priority() const { return priority_; }
   PartialData* partial() { return partial_.get(); }
   bool is_truncated() { return truncated_; }
diff --git a/net/http/http_transaction_test_util.cc b/net/http/http_transaction_test_util.cc
index 646dde5..88a416c 100644
--- a/net/http/http_transaction_test_util.cc
+++ b/net/http/http_transaction_test_util.cc
@@ -389,6 +389,10 @@
   const MockTransaction* t = FindMockTransaction(current_request_.url);
   DCHECK(t);
 
+  if (test_mode_ & TEST_MODE_STUCK_READ) {
+    return ERR_IO_PENDING;
+  }
+
   CHECK(!done_reading_called_);
   reading_ = true;
 
@@ -517,8 +521,17 @@
   if (t->start_handler) {
     DCHECK_EQ(start_return_code, OK)
         << "set either start_return_code or start_handler, not both";
-    start_return_code = t->start_handler.Run(&current_request_);
+    start_return_code = t->start_handler.Run(
+        &current_request_, base::BindOnce(&MockNetworkTransaction::OnIOComplete,
+                                          weak_factory_.GetWeakPtr()));
+    if (start_return_code == ERR_IO_PENDING) {
+      // The `start_handler` is responsible for resuming us.
+      callback_ = std::move(callback);
+      next_state_ = State::CREATE_STREAM;
+      return ERR_IO_PENDING;
+    }
   }
+
   if (OK != start_return_code) {
     if (test_mode_ & TEST_MODE_SYNC_NET_START) {
       return start_return_code;
diff --git a/net/http/http_transaction_test_util.h b/net/http/http_transaction_test_util.h
index 3ce1625..c830d6c 100644
--- a/net/http/http_transaction_test_util.h
+++ b/net/http/http_transaction_test_util.h
@@ -52,14 +52,15 @@
 enum {
   TEST_MODE_NORMAL = 0,
   TEST_MODE_SYNC_NET_START = 1 << 0,
-  TEST_MODE_SYNC_NET_READ  = 1 << 1,
+  TEST_MODE_SYNC_NET_READ = 1 << 1,
   TEST_MODE_SYNC_CACHE_START = 1 << 2,
-  TEST_MODE_SYNC_CACHE_READ  = 1 << 3,
-  TEST_MODE_SYNC_CACHE_WRITE  = 1 << 4,
+  TEST_MODE_SYNC_CACHE_READ = 1 << 3,
+  TEST_MODE_SYNC_CACHE_WRITE = 1 << 4,
   TEST_MODE_SYNC_ALL = (TEST_MODE_SYNC_NET_START | TEST_MODE_SYNC_NET_READ |
                         TEST_MODE_SYNC_CACHE_START | TEST_MODE_SYNC_CACHE_READ |
                         TEST_MODE_SYNC_CACHE_WRITE),
-  TEST_MODE_SLOW_READ = 1 << 5
+  TEST_MODE_SLOW_READ = 1 << 5,
+  TEST_MODE_STUCK_READ = 1 << 6
 };
 
 using MockTransactionReadHandler = base::RepeatingCallback<
@@ -71,7 +72,8 @@
                                  std::string* response_headers,
                                  std::string* response_data)>;
 using MockTransactionStartHandler =
-    base::RepeatingCallback<Error(const HttpRequestInfo* request)>;
+    base::RepeatingCallback<Error(const HttpRequestInfo* request,
+                                  CompletionOnceCallback callback)>;
 
 // Default TransportInfo suitable for most MockTransactions.
 // Describes a direct connection to (127.0.0.1, 80).
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 0943611c..d15b408 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -10821,7 +10821,8 @@
 
   transaction.request_headers = EXTRA_HEADER;
   transaction.start_handler =
-      base::BindLambdaForTesting([&](const HttpRequestInfo* request) -> Error {
+      base::BindLambdaForTesting([&](const HttpRequestInfo* request,
+                                     CompletionOnceCallback callback) -> Error {
         std::optional<std::string_view> range_header =
             request->extra_headers.GetHeaderView(HttpRequestHeaders::kRange);
         if (!range_header) {
@@ -17493,6 +17494,78 @@
             base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
 };
 
+// Test for cache timeout triggering at strange time when a transaction
+// already had a restart queued since saved entry's validation failed.
+//
+// See crbug.com/569674221
+TEST_F(HttpCacheTestWithMockTime, LockTimeoutRace) {
+  MockHttpCache cache;
+
+  ScopedMockTransaction transaction(kSimpleGET_Transaction);
+  transaction.response_headers = "Cache-Control: max-age=0;\n ETag: 'a'\n";
+  transaction.test_mode = TEST_MODE_STUCK_READ;
+
+  // Request 1: gets stuck reading data.
+  MockHttpRequest request1(transaction);
+  std::unique_ptr<HttpTransaction> trans_stuck_in_data_phase;
+  {
+    TestCompletionCallback callback;
+    trans_stuck_in_data_phase = cache.CreateTransaction();
+
+    int rv = callback.GetResult(trans_stuck_in_data_phase->Start(
+        &request1, callback.callback(), NetLogWithSource()));
+    ASSERT_EQ(rv, OK);
+
+    TestCompletionCallback read_callback;
+    auto buf = base::MakeRefCounted<IOBufferWithSize>(256);
+    rv = trans_stuck_in_data_phase->Read(buf.get(), 256,
+                                         read_callback.callback());
+    ASSERT_EQ(rv, ERR_IO_PENDING);
+  }
+
+  // Request 2: paused in sending request; will try (and fail) validation
+  // once resumed.
+  base::RunLoop run_loop;
+  CompletionOnceCallback concurrent1_callback;
+  transaction.start_handler =
+      base::BindLambdaForTesting([&](const HttpRequestInfo* request,
+                                     CompletionOnceCallback callback) -> Error {
+        concurrent1_callback = std::move(callback);
+        run_loop.Quit();
+        return ERR_IO_PENDING;
+      });
+
+  MockHttpRequest request2(transaction);
+  std::unique_ptr<HttpTransaction> trans_concurrent_1;
+
+  TestCompletionCallback callback2;
+  trans_concurrent_1 = cache.CreateTransaction();
+
+  int rv = trans_concurrent_1->Start(&request2, callback2.callback(),
+                                     NetLogWithSource());
+  ASSERT_EQ(rv, ERR_IO_PENDING);
+
+  run_loop.Run();
+
+  // Request 3: to same URL; we resume request 2 once it returns from
+  // Start when it should be trying to be added to the entry.
+  MockHttpRequest request3(transaction);
+  std::unique_ptr<HttpTransaction> trans_concurrent_2;
+
+  TestCompletionCallback callback3;
+  trans_concurrent_2 = cache.CreateTransaction();
+
+  rv = trans_concurrent_2->Start(&request3, callback3.callback(),
+                                 NetLogWithSource());
+  ASSERT_EQ(rv, ERR_IO_PENDING);
+  std::move(concurrent1_callback).Run(net::OK);
+
+  // Advance past the timeline for cache lock, so it can beat the
+  // restart.
+  AdvanceClock(base::Seconds(30));
+  EXPECT_EQ(OK, callback2.WaitForResult());
+}
+
 TEST_F(HttpCacheTestWithMockTime, InvalidationFilterTimeBoundaries) {
   AddScopedFeatureList().InitAndEnableFeature(
       net::features::kLogicalClearHttpCache);