Blob Blame History Raw
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 " <<