You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jv...@apache.org on 2026/10/19 22:41:14 UTC

(trafficserver) branch master updated: Add cache test for closing read mid-object (#13818)

This is an automated email from the ASF dual-hosted git repository.

JosiahWI pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 97d2831fbd Add cache test for closing read mid-object (#13818)
97d2831fbd is described below

commit 97d2831fbd38c8f43b39e3ed996d9bd6e3173384
Author: JosiahWI <41...@users.noreply.github.com>
AuthorDate: Mon Oct 19 17:41:08 2026 -0500

    Add cache test for closing read mid-object (#13818)
    
    This patch extends the cache test coverage to ease future refactoring.
---
 src/iocore/cache/CMakeLists.txt                    |   1 +
 .../unit_tests/test_Read_Close_After_Ready.cc      | 215 +++++++++++++++++++++
 2 files changed, 216 insertions(+)

diff --git a/src/iocore/cache/CMakeLists.txt b/src/iocore/cache/CMakeLists.txt
index 3a8865baf4..c0a7649cb3 100644
--- a/src/iocore/cache/CMakeLists.txt
+++ b/src/iocore/cache/CMakeLists.txt
@@ -87,6 +87,7 @@ if(BUILD_TESTING)
   add_cache_test(CacheVol unit_tests/test_CacheVol.cc)
   add_cache_test(StripeHashTable unit_tests/test_StripeHashTable.cc)
   add_cache_test(RWW unit_tests/test_RWW.cc)
+  add_cache_test(Read_Close_After_Ready unit_tests/test_Read_Close_After_Ready.cc)
   add_cache_test(Alternate_L_to_S unit_tests/test_Alternate_L_to_S.cc)
   add_cache_test(Alternate_S_to_L unit_tests/test_Alternate_S_to_L.cc)
   add_cache_test(Alternate_L_to_S_remove_L unit_tests/test_Alternate_L_to_S_remove_L.cc)
diff --git a/src/iocore/cache/unit_tests/test_Read_Close_After_Ready.cc b/src/iocore/cache/unit_tests/test_Read_Close_After_Ready.cc
new file mode 100644
index 0000000000..457e958b2a
--- /dev/null
+++ b/src/iocore/cache/unit_tests/test_Read_Close_After_Ready.cc
@@ -0,0 +1,215 @@
+/** @file
+
+  Closing a cache reader after its first READ_READY.
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+
+#include "main.h"
+
+#include <cstddef>
+#include <memory>
+
+int  cache_vols           = 1;
+bool reuse_existing_cache = false;
+
+namespace
+{
+
+constexpr std::size_t LARGE_FILE   = 10 * 1024 * 1024;
+constexpr char        OBJECT_URL[] = "http://www.close-during-read.com/";
+
+class WriteObject : public CacheTestHandler
+{
+public:
+  WriteObject(std::size_t size, const char *url) : CacheTestHandler()
+  {
+    this->_wt        = new CacheWriteTest(size, this, url);
+    this->_wt->mutex = this->mutex;
+
+    SET_HANDLER(&WriteObject::start_test);
+  }
+
+  int
+  start_test(int /* event ATS_UNUSED */, void * /* e ATS_UNUSED */)
+  {
+    this_ethread()->schedule_imm(this->_wt);
+    return 0;
+  }
+
+  void
+  handle_cache_event(int event, CacheTestBase *base) override
+  {
+    switch (event) {
+    case CACHE_EVENT_OPEN_WRITE:
+      base->do_io_write();
+      break;
+    case VC_EVENT_WRITE_READY:
+      base->reenable();
+      break;
+    case VC_EVENT_WRITE_COMPLETE:
+      base->close();
+      delete this;
+      break;
+    default:
+      FAIL("unexpected event " << event << " while writing the object");
+      break;
+    }
+  }
+};
+
+class CloseReadAfterReady : public CacheTestHandler
+{
+public:
+  CloseReadAfterReady(std::size_t size, const char *url) : CacheTestHandler(), _size(size), _url(url)
+  {
+    this->_rt        = new CacheReadTest(size, this, url);
+    this->_rt->mutex = this->mutex;
+
+    SET_HANDLER(&CloseReadAfterReady::start_test);
+  }
+
+  ~CloseReadAfterReady() override { delete this->_closed_reader; }
+
+  int
+  start_test(int /* event ATS_UNUSED */, void * /* e ATS_UNUSED */)
+  {
+    this_ethread()->schedule_imm(this->_rt);
+    return 0;
+  }
+
+  // Closing from a separate event, after the READ_READY callback has
+  // returned, aims to close while the cache may be reading the next fragment.
+  int
+  close_reader(int /* event ATS_UNUSED */, void * /* e ATS_UNUSED */)
+  {
+    if (this->_read_completed_before_close) {
+      return 0;
+    }
+
+    this->_rt->vc->do_io_close();
+    this->_rt->vc        = nullptr;
+    this->_rt->vio       = nullptr;
+    this->_closed_reader = this->_rt;
+
+    this->_rt        = new CacheReadTest(this->_size, this, this->_url);
+    this->_rt->mutex = this->mutex;
+    this_ethread()->schedule_imm(this->_rt);
+    return 0;
+  }
+
+  void
+  handle_cache_event(int event, CacheTestBase *base) override
+  {
+    if (base == this->_closed_reader) {
+      ++this->events_after_close;
+      return;
+    }
+
+    switch (event) {
+    case CACHE_EVENT_OPEN_READ_RWW:
+      break;
+    case CACHE_EVENT_OPEN_READ:
+      base->do_io_read();
+      break;
+    case VC_EVENT_READ_READY:
+      on_read_ready(base);
+      break;
+    case VC_EVENT_READ_COMPLETE:
+      if (this->_closed_reader != nullptr) {
+        this->reread_completed = true;
+      } else {
+        this->_read_completed_before_close = true;
+      }
+      base->close();
+      finish();
+      break;
+    default:
+      FAIL("unexpected event " << event << " while reading the object");
+      break;
+    }
+  }
+
+  int  events_after_close = 0;
+  bool reread_completed   = false;
+
+private:
+  // The test case owns the spy, so hand off to the next test without deleting.
+  void
+  finish()
+  {
+    this->next_test();
+    this->next = nullptr;
+  }
+
+  void
+  on_read_ready(CacheTestBase *base)
+  {
+    if (this->_closed_reader != nullptr) {
+      base->reenable();
+    } else if (!this->_close_scheduled) {
+      this->_close_scheduled = true;
+      SET_HANDLER(&CloseReadAfterReady::close_reader);
+      this_ethread()->schedule_imm(this);
+    }
+  }
+
+  std::size_t    _size;
+  const char    *_url;
+  bool           _close_scheduled             = false;
+  bool           _read_completed_before_close = false;
+  CacheTestBase *_closed_reader               = nullptr;
+};
+
+class CloseAfterReadyInit : public CacheInit
+{
+public:
+  explicit CloseAfterReadyInit(CloseReadAfterReady *spy) : _spy(spy) {}
+
+  int
+  cache_init_success_callback(int /* event ATS_UNUSED */, void * /* e ATS_UNUSED */) override
+  {
+    auto *write = new WriteObject(LARGE_FILE, OBJECT_URL);
+
+    write->add(this->_spy);
+    write->add(new TerminalTest);
+    this_ethread()->schedule_imm(write);
+    delete this;
+    return 0;
+  }
+
+private:
+  CloseReadAfterReady *_spy;
+};
+
+} // namespace
+
+TEST_CASE("Closing a cache reader after its first READ_READY delivers no further events to it and leaves the object readable",
+          "cache")
+{
+  init_cache(256 * 1024 * 1024);
+
+  auto spy = std::make_unique<CloseReadAfterReady>(LARGE_FILE, OBJECT_URL);
+
+  this_ethread()->schedule_imm(new CloseAfterReadyInit{spy.get()});
+  this_thread()->execute();
+
+  CHECK(spy->events_after_close == 0);
+  CHECK(spy->reread_completed);
+}