Avoid memory corruption and segfaults during ufs cache store maintenance
by removing being-deleted StoreEntry from ufs cache replacement policies.

SMP Store changes added a SwapDir::disconnect() method that notified
entry's cache_dir that its entry is being destroyed. I added that code so that
shared cache_dirs can update their index when idle store_table entries are
destroyed on cleanup (completely avoiding idle entries proved to be too
difficult with store_table still around).

However, I did not realize that non-shared cache_dirs have the same problem:
Their StoreEntries may be deleted without cache_dirs knowing that. UFS
cache_dir now implement the SwapDir::disconnect() API to stay in sync. It is
likely that the problem was introduced by SMP Store changes, but I do not know
whether there is a better fix for it.

I also added assertions to catch cases where a dirty StoreEntry or MemObject
(i.e., an entry or object that is still in some replacement policy index) is
being destroyed.

TODO: This patch breaks "make check". We need more STUBs.

=== modified file 'src/MemObject.cc'
--- src/MemObject.cc	2012-07-20 12:44:39 +0000
+++ src/MemObject.cc	2012-07-25 23:37:21 +0000
@@ -101,40 +101,42 @@
 
     /* XXX account log_url */
 
     swapout.decision = SwapOut::swNeedsCheck;
 }
 
 MemObject::~MemObject()
 {
     debugs(20, 3, HERE << "del MemObject " << this);
     const Ctx ctx = ctx_enter(url);
 #if URL_CHECKSUM_DEBUG
 
     assert(chksum == url_checksum(url));
 #endif
 
     if (!shutting_down)
         assert(swapout.sio == NULL);
 
     data_hdr.freeContent();
 
+    assert(!repl.data); // we are not in some replacement policy queue
+
 #if 0
     /*
      * There is no way to abort FD-less clients, so they might
      * still have mem->clients set.
      */
     assert(clients.head == NULL);
 
 #endif
 
     HTTPMSGUNLOCK(_reply);
 
     HTTPMSGUNLOCK(request);
 
     ctx_exit(ctx);              /* must exit before we free mem->url */
 
     safe_free(url);
 
     safe_free(log_url);    /* XXX account log_url */
 
     safe_free(vary_headers);

=== modified file 'src/SwapDir.h'
--- src/SwapDir.h	2012-07-13 14:33:19 +0000
+++ src/SwapDir.h	2012-07-26 00:01:59 +0000
@@ -142,41 +142,41 @@
     virtual void diskFull();
 
     virtual StoreEntry * get(const cache_key *);
 
     virtual void get(String const, STOREGETCLIENT, void * cbdata);
 
     virtual uint64_t maxSize() const { return max_size;}
 
     virtual uint64_t minSize() const;
 
     virtual int64_t maxObjectSize() const { return max_objsize; }
 
     virtual void getStats(StoreInfoStats &stats) const;
     virtual void stat (StoreEntry &anEntry) const;
     virtual StoreSearch *search(String const url, HttpRequest *) = 0;
 
     /* migrated from store_dir.cc */
     bool objectSizeIsAcceptable(int64_t objsize) const;
 
     /// called when the entry is about to forget its association with cache_dir
-    virtual void disconnect(StoreEntry &) {}
+    virtual void disconnect(StoreEntry &e) = 0;
 
     /// called when entry swap out is complete
     virtual void swappedOut(const StoreEntry &e) = 0;
 
 protected:
     void parseOptions(int reconfiguring);
     void dumpOptions(StoreEntry * e) const;
     virtual ConfigOption *getOptionTree() const;
     virtual bool allowOptionReconfigure(const char *const) const { return true; }
 
     int64_t sizeInBlocks(const int64_t size) const { return (size + fs.blksize - 1) / fs.blksize; }
 
 private:
     bool optionReadOnlyParse(char const *option, const char *value, int reconfiguring);
     void optionReadOnlyDump(StoreEntry * e) const;
     bool optionObjectSizeParse(char const *option, const char *value, int reconfiguring);
     void optionObjectSizeDump(StoreEntry * e) const;
     char const *theType;
 
 protected:

=== modified file 'src/fs/ufs/store_dir_ufs.cc'
--- src/fs/ufs/store_dir_ufs.cc	2012-07-23 15:34:12 +0000
+++ src/fs/ufs/store_dir_ufs.cc	2012-07-26 00:18:20 +0000
@@ -420,40 +420,60 @@
     if (repl->Referenced)
         repl->Referenced(repl, &e, &e.repl);
 }
 
 /*
  * UFSSwapDir::dereference
  * This routine is called whenever the last reference to an object is
  * removed, to maintain replacement information within the storage fs.
  */
 bool
 UFSSwapDir::dereference(StoreEntry & e)
 {
     debugs(47, 3, "UFSSwapDir::dereference: referencing " << &e << " " << e.swap_dirn << "/" << e.swap_filen);
 
     if (repl->Dereferenced)
         repl->Dereferenced(repl, &e, &e.repl);
 
     return true; // keep e in the global store_table
 }
 
+void
+UFSSwapDir::disconnect(StoreEntry &e)
+{
+    assert(e.swap_dirn == index);
+    assert(e.swap_filen >= 0);
+
+    // We do not want to unlink(e) in this low-level method because it is
+    // called during Squid shutdown sequence (among other times).
+    if (e.swap_status == SWAPOUT_DONE) {
+        cur_size -= fs.blksize * sizeInBlocks(e.swap_file_sz);
+        --n_disk_objects;
+    }
+    replacementRemove(&e);
+    mapBitReset(e.swap_filen);
+
+    e.swap_dirn = -1;
+    e.swap_filen = -1;
+    e.swap_status = SWAPOUT_NONE;
+}
+
 StoreIOState::Pointer
 UFSSwapDir::createStoreIO(StoreEntry &e, StoreIOState::STFNCB * file_callback, StoreIOState::STIOCB * aCallback, void *callback_data)
 {
     return IO->create (this, &e, file_callback, aCallback, callback_data);
 }
 
 StoreIOState::Pointer
 UFSSwapDir::openStoreIO(StoreEntry &e, StoreIOState::STFNCB * file_callback, StoreIOState::STIOCB * aCallback, void *callback_data)
 {
     return IO->open (this, &e, file_callback, aCallback, callback_data);
 }
 
 int
 UFSSwapDir::mapBitTest(sfileno filn)
 {
     return map->testBit(filn);
 }
 
 void
 UFSSwapDir::mapBitSet(sfileno filn)

=== modified file 'src/fs/ufs/ufscommon.h'
--- src/fs/ufs/ufscommon.h	2012-07-20 23:11:02 +0000
+++ src/fs/ufs/ufscommon.h	2012-07-26 00:02:33 +0000
@@ -50,40 +50,41 @@
 
 public:
     static int IsUFSDir(SwapDir* sd);
     static int DirClean(int swap_index);
     static int FilenoBelongsHere(int fn, int F0, int F1, int F2);
 
     UFSSwapDir(char const *aType, const char *aModuleType);
     virtual void init();
     virtual void create();
     virtual void dump(StoreEntry &) const;
     ~UFSSwapDir();
     virtual StoreSearch *search(String const url, HttpRequest *);
     virtual bool doubleCheck(StoreEntry &);
     virtual bool unlinkdUseful() const;
     virtual void unlink(StoreEntry &);
     virtual void statfs(StoreEntry &)const;
     virtual void maintain();
     virtual bool canStore(const StoreEntry &e, int64_t diskSpaceNeeded, int &load) const;
     virtual void reference(StoreEntry &);
     virtual bool dereference(StoreEntry &);
+    virtual void disconnect(StoreEntry &e);
     virtual StoreIOState::Pointer createStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
     virtual StoreIOState::Pointer openStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
     virtual void openLog();
     virtual void closeLog();
     virtual int writeCleanStart();
     virtual void writeCleanDone();
     virtual void logEntry(const StoreEntry & e, int op) const;
     virtual void parse(int index, char *path);
     virtual void reconfigure();
     virtual int callback();
     virtual void sync();
     virtual void swappedOut(const StoreEntry &e);
     virtual uint64_t currentSize() const { return cur_size; }
     virtual uint64_t currentCount() const { return n_disk_objects; }
 
     void unlinkFile(sfileno f);
     // move down when unlink is a virtual method
     //protected:
     UFSStrategy *IO;
     char *fullPath(sfileno, char *) const;

=== modified file 'src/store.cc'
--- src/store.cc	2012-07-23 07:02:06 +0000
+++ src/store.cc	2012-07-25 23:37:21 +0000
@@ -386,40 +386,42 @@
         hidden_mem_obj(NULL),
         swap_file_sz(0)
 {
     debugs(20, 3, HERE << "new StoreEntry " << this);
     mem_obj = new MemObject(aUrl, aLogUrl);
 
     expires = lastmod = lastref = timestamp = -1;
 
     swap_status = SWAPOUT_NONE;
     swap_filen = -1;
     swap_dirn = -1;
 }
 
 StoreEntry::~StoreEntry()
 {
     if (swap_filen >= 0) {
         SwapDir &sd = dynamic_cast<SwapDir&>(*store());
         sd.disconnect(*this);
     }
     delete hidden_mem_obj;
+    assert(!mem_obj);
+    assert(!repl.data);
 }
 
 #if USE_ADAPTATION
 void
 StoreEntry::deferProducer(const AsyncCall::Pointer &producer)
 {
     if (!deferredProducer)
         deferredProducer = producer;
     else
         debugs(20, 5, HERE << "Deferred producer call is allready set to: " <<
                *deferredProducer << ", requested call: " << *producer);
 }
 
 void
 StoreEntry::kickProducer()
 {
     if (deferredProducer != NULL) {
         ScheduleCallHere(deferredProducer);
         deferredProducer = NULL;
     }
@@ -434,42 +436,42 @@
     MemObject *mem = mem_obj;
     mem_obj = NULL;
     delete mem;
     delete hidden_mem_obj;
     hidden_mem_obj = NULL;
 }
 
 void
 StoreEntry::hideMemObject()
 {
     debugs(20, 3, HERE << "hiding " << mem_obj);
     assert(mem_obj);
     assert(!hidden_mem_obj);
     hidden_mem_obj = mem_obj;
     mem_obj = NULL;
 }
 
 void
 destroyStoreEntry(void *data)
 {
-    debugs(20, 3, HERE << "destroyStoreEntry: destroying " <<  data);
     StoreEntry *e = static_cast<StoreEntry *>(static_cast<hash_link *>(data));
+    debugs(20, 3, HERE << "destroyStoreEntry: destroying " << e << " or " << data);
     assert(e != NULL);
 
     if (e == NullStoreEntry::getInstance())
         return;
 
     e->destroyMemObject();
 
     e->hashDelete();
 
     assert(e->key == NULL);
 
     delete e;
 }
 
 /* ----- INTERFACE BETWEEN STORAGE MANAGER AND HASH TABLE FUNCTIONS --------- */
 
 void
 StoreEntry::hashInsert(const cache_key * someKey)
 {
     debugs(20, 3, "StoreEntry::hashInsert: Inserting Entry " << this << " key '" << storeKeyText(someKey) << "'");