From 22cf01808027242b7e8beee3f1c8d4d64a25af2a Mon Sep 17 00:00:00 2001 From: Lubos Uhliarik Date: Mon, 29 Apr 2019 10:07:21 +0000 Subject: [PATCH] Resolves: #1599074 - squid: 3 coredumps every day --- squid-4.6-swapout-failure.patch | 194 ++++++++++++++++++++++++++++++++ squid.spec | 7 +- 2 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 squid-4.6-swapout-failure.patch diff --git a/squid-4.6-swapout-failure.patch b/squid-4.6-swapout-failure.patch new file mode 100644 index 0000000..03abe53 --- /dev/null +++ b/squid-4.6-swapout-failure.patch @@ -0,0 +1,194 @@ +diff --git a/src/Store.h b/src/Store.h +index 552695d..84b384a 100644 +--- a/src/Store.h ++++ b/src/Store.h +@@ -119,6 +119,8 @@ public: + bool swappingOut() const { return swap_status == SWAPOUT_WRITING; } + /// whether the entire entry is now on disk (possibly marked for deletion) + bool swappedOut() const { return swap_status == SWAPOUT_DONE; } ++ /// whether we failed to write this entry to disk ++ bool swapoutFailed() const { return swap_status == SWAPOUT_FAILED; } + void swapOutFileClose(int how); + const char *url() const; + /// Satisfies cachability requirements shared among disk and RAM caches. +diff --git a/src/enums.h b/src/enums.h +index 6931def..daf626d 100644 +--- a/src/enums.h ++++ b/src/enums.h +@@ -57,7 +57,11 @@ typedef enum { + SWAPOUT_WRITING, + /// StoreEntry is associated with a complete (i.e., fully swapped out) disk store entry. + /// Guarantees the disk store entry existence. +- SWAPOUT_DONE ++ SWAPOUT_DONE, ++ /// StoreEntry is associated with an unusable disk store entry. ++ /// Swapout attempt has failed. The entry should be marked for eventual deletion. ++ /// Guarantees the disk store entry existence. ++ SWAPOUT_FAILED + } swap_status_t; + + typedef enum { +diff --git a/src/fs/ufs/UFSSwapDir.cc b/src/fs/ufs/UFSSwapDir.cc +index 3efefe1..9651252 100644 +--- a/src/fs/ufs/UFSSwapDir.cc ++++ b/src/fs/ufs/UFSSwapDir.cc +@@ -1181,6 +1181,8 @@ Fs::Ufs::UFSSwapDir::evictCached(StoreEntry & e) + if (!e.hasDisk()) + return; // see evictIfFound() + ++ // Since these fields grow only after swap out ends successfully, ++ // do not decrement them for e.swappingOut() and e.swapoutFailed(). + if (e.swappedOut()) { + cur_size -= fs.blksize * sizeInBlocks(e.swap_file_sz); + --n_disk_objects; +@@ -1270,7 +1272,7 @@ void + Fs::Ufs::UFSSwapDir::finalizeSwapoutFailure(StoreEntry &entry) + { + debugs(47, 5, entry); +- // rely on the expected subsequent StoreEntry::release(), evictCached(), or ++ // rely on the expected eventual StoreEntry::release(), evictCached(), or + // a similar call to call unlink(), detachFromDisk(), etc. for the entry. + } + +diff --git a/src/store.cc b/src/store.cc +index 0a4748c..699dbf8 100644 +--- a/src/store.cc ++++ b/src/store.cc +@@ -83,7 +83,8 @@ const char *storeStatusStr[] = { + const char *swapStatusStr[] = { + "SWAPOUT_NONE", + "SWAPOUT_WRITING", +- "SWAPOUT_DONE" ++ "SWAPOUT_DONE", ++ "SWAPOUT_FAILED" + }; + + /* +@@ -257,6 +258,8 @@ StoreEntry::setNoDelay(bool const newValue) + // XXX: Type names mislead. STORE_DISK_CLIENT actually means that we should + // open swapin file, aggressively trim memory, and ignore read-ahead gap. + // It does not mean we will read from disk exclusively (or at all!). ++// STORE_MEM_CLIENT covers all other cases, including in-memory entries, ++// newly created entries, and entries not backed by disk or memory cache. + // XXX: May create STORE_DISK_CLIENT with no disk caching configured. + // XXX: Collapsed clients cannot predict their type. + store_client_t +@@ -279,6 +282,9 @@ StoreEntry::storeClientType() const + return STORE_MEM_CLIENT; + } + ++ if (swapoutFailed()) ++ return STORE_MEM_CLIENT; ++ + if (store_status == STORE_OK) { + /* the object has completed. */ + +@@ -2044,13 +2050,23 @@ StoreEntry::detachFromDisk() + void + StoreEntry::checkDisk() const + { +- const bool ok = (swap_dirn < 0) == (swap_filen < 0) && +- (swap_dirn < 0) == (swap_status == SWAPOUT_NONE) && +- (swap_dirn < 0 || swap_dirn < Config.cacheSwap.n_configured); +- +- if (!ok) { +- debugs(88, DBG_IMPORTANT, "ERROR: inconsistent disk entry state " << *this); +- throw std::runtime_error("inconsistent disk entry state "); ++ try { ++ if (swap_dirn < 0) { ++ Must(swap_filen < 0); ++ Must(swap_status == SWAPOUT_NONE); ++ } else { ++ Must(swap_filen >= 0); ++ Must(swap_dirn < Config.cacheSwap.n_configured); ++ if (swapoutFailed()) { ++ Must(EBIT_TEST(flags, RELEASE_REQUEST)); ++ } else { ++ Must(swappingOut() || swappedOut()); ++ } ++ } ++ } catch (...) { ++ debugs(88, DBG_IMPORTANT, "ERROR: inconsistent disk entry state " << ++ *this << "; problem: " << CurrentException); ++ throw; + } + } + +diff --git a/src/store_client.cc b/src/store_client.cc +index c372b63..49f9d32 100644 +--- a/src/store_client.cc ++++ b/src/store_client.cc +@@ -162,7 +162,7 @@ store_client::store_client(StoreEntry *e) : + if (getType() == STORE_DISK_CLIENT) { + /* assert we'll be able to get the data we want */ + /* maybe we should open swapin_sio here */ +- assert(entry->hasDisk() || entry->swappingOut()); ++ assert(entry->hasDisk() && !entry->swapoutFailed()); + } + } + +@@ -662,7 +662,8 @@ storeUnregister(store_client * sc, StoreEntry * e, void *data) + dlinkDelete(&sc->node, &mem->clients); + -- mem->nclients; + +- if (e->store_status == STORE_OK && !e->swappedOut()) ++ const auto swapoutFinished = e->swappedOut() || e->swapoutFailed(); ++ if (e->store_status == STORE_OK && !swapoutFinished) + e->swapOut(); + + if (sc->swapin_sio != NULL) { +diff --git a/src/store_swapin.cc b/src/store_swapin.cc +index 4f3d7f6..0cff0d0 100644 +--- a/src/store_swapin.cc ++++ b/src/store_swapin.cc +@@ -38,6 +38,11 @@ storeSwapInStart(store_client * sc) + return; + } + ++ if (e->swapoutFailed()) { ++ debugs(20, DBG_IMPORTANT, "BUG: Attempt to swap in a failed-to-store entry " << *e << ". Salvaged."); ++ return; ++ } ++ + assert(e->mem_obj != NULL); + sc->swapin_sio = storeOpen(e, storeSwapInFileNotify, storeSwapInFileClosed, sc); + } +diff --git a/src/store_swapout.cc b/src/store_swapout.cc +index ad71993..5761198 100644 +--- a/src/store_swapout.cc ++++ b/src/store_swapout.cc +@@ -88,19 +88,9 @@ storeSwapOutStart(StoreEntry * e) + + /// XXX: unused, see a related StoreIOState::file_callback + static void +-storeSwapOutFileNotify(void *data, int errflag, StoreIOState::Pointer self) ++storeSwapOutFileNotify(void *, int, StoreIOState::Pointer) + { +- StoreEntry *e; +- static_cast(data)->unwrap(&e); +- +- MemObject *mem = e->mem_obj; +- assert(e->swappingOut()); +- assert(mem); +- assert(mem->swapout.sio == self); +- assert(errflag == 0); +- assert(!e->hasDisk()); // if this fails, call SwapDir::disconnect(e) +- e->swap_filen = mem->swapout.sio->swap_filen; +- e->swap_dirn = mem->swapout.sio->swap_dirn; ++ assert(false); + } + + static bool +@@ -304,8 +294,11 @@ storeSwapOutFileClosed(void *data, int errflag, StoreIOState::Pointer self) + storeConfigure(); + } + ++ // mark the locked entry for deletion ++ // TODO: Keep the memory entry (if any) ++ e->releaseRequest(); ++ e->swap_status = SWAPOUT_FAILED; + e->disk().finalizeSwapoutFailure(*e); +- e->releaseRequest(); // TODO: Keep the memory entry (if any) + } else { + /* swapping complete */ + debugs(20, 3, "storeSwapOutFileClosed: SwapOut complete: '" << e->url() << "' to " << diff --git a/squid.spec b/squid.spec index 0d18221..793a28b 100644 --- a/squid.spec +++ b/squid.spec @@ -2,7 +2,7 @@ Name: squid Version: 4.6 -Release: 1%{?dist} +Release: 2%{?dist} Summary: The Squid proxy caching server Epoch: 7 # See CREDITS for breakdown of non GPLv2+ code @@ -32,6 +32,7 @@ Patch202: squid-3.1.0.9-location.patch Patch203: squid-3.0.STABLE1-perlpath.patch Patch204: squid-3.5.9-include-guards.patch Patch205: squid-4.0.21-large-acl.patch +Patch206: squid-4.6-swapout-failure.patch Requires: bash >= 2.0 Requires(pre): shadow-utils @@ -87,6 +88,7 @@ lookup program (dnsserver), a program for retrieving FTP data %patch203 -p1 -b .perlpath %patch204 -p0 -b .include-guards %patch205 -p1 -b .large_acl +%patch206 -p1 -b .swapout-failure %build # cppunit-config patch changes configure.ac @@ -290,6 +292,9 @@ fi %changelog +* Mon Apr 29 2019 Lubos Uhliarik - 7:4.6-2 +- Resolves: #1599074 - squid: 3 coredumps every day + * Wed Apr 24 2019 Lubos Uhliarik - 7:4.6-1 - new version 4.6 - disabled strict checking due to gcc warnings