rpms/squid
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Resolves: #1599074 - squid: 3 coredumps every day

Browse Source
This commit is contained in:
Lubos Uhliarik 2019-04-29 10:07:21 +00:00
parent a942cf4117
commit 22cf018080
2 changed files with 200 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

194
squid-4.6-swapout-failure.patch Normal file
View File

@ -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<generic_cbdata *>(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 " <<

7
squid.spec
View File

@ -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 <luhliari@redhat.com> - 7:4.6-2
- Resolves: #1599074 - squid: 3 coredumps every day
* Wed Apr 24 2019 Lubos Uhliarik <luhliari@redhat.com> - 7:4.6-1
- new version 4.6
- disabled strict checking due to gcc warnings