From e877aaac11e7c6c5379d85ea2f6ad86151b0c654 Mon Sep 17 00:00:00 2001 From: wessels <> Date: Tue, 23 May 2006 01:58:51 +0000 Subject: [PATCH] Merged storeIOState and StoreIOState into a single StoreIOState class. Previously, StoreIOState was just a refcounted storeIOState. I had concerns that mixed use of refcounted and non-refcounted StoreIOState may be leading to cases where the object gets referenced via the old-style pointer after the refcounted version had its count go to zero and the memory was freed. This came about while investigating bugzilla #1465. removed StoreIOState-related prototypes from protos.h, and removed some typedefs from typedefs.h. Modified STFNCB (storeSwapInFileNotify, storeSwapOutFileNotify) and STIOCB (storeSwapOutFileClosed, storeSwapInFileClosed) so that they no longer take StoreIOState arguments. Their "data" arguments easily lead to the corresponding StoreIOState. The only thing we really lose here is the ability to assert that "data->sio" equals the passed StoreIOState. --- src/DiskIO/DiskDaemon/DiskdIOStrategy.h | 5 ++- src/MemObject.h | 3 +- src/StoreClient.h | 3 +- src/StoreIOState.cc | 12 +++--- src/StoreIOState.h | 51 +++++++++++++++++++------ src/fs/coss/CossSwapDir.h | 2 +- src/fs/coss/store_coss.h | 2 +- src/fs/ufs/store_io_ufs.cc | 4 +- src/fs/ufs/ufscommon.h | 4 +- src/protos.h | 10 +---- src/store_io.cc | 2 +- src/store_swapin.cc | 15 ++++---- src/store_swapout.cc | 10 ++--- src/tests/TestSwapDir.h | 8 ++-- src/typedefs.h | 8 +--- 15 files changed, 78 insertions(+), 61 deletions(-) diff --git a/src/DiskIO/DiskDaemon/DiskdIOStrategy.h b/src/DiskIO/DiskDaemon/DiskdIOStrategy.h index 2873a07ab8..b1f5b49cec 100644 --- a/src/DiskIO/DiskDaemon/DiskdIOStrategy.h +++ b/src/DiskIO/DiskDaemon/DiskdIOStrategy.h @@ -1,6 +1,6 @@ /* - * $Id: DiskdIOStrategy.h,v 1.1 2004/12/20 16:30:38 robertc Exp $ + * $Id: DiskdIOStrategy.h,v 1.2 2006/05/22 19:58:51 wessels Exp $ * * DEBUG: section 79 Squid-side DISKD I/O functions. * AUTHOR: Duane Wessels @@ -68,6 +68,7 @@ public: }; #include "DiskIO/DiskIOStrategy.h" +#include "StoreIOState.h" class DiskFile; @@ -101,7 +102,7 @@ private: void optionQ1Dump(StoreEntry * e) const; bool optionQ2Parse(char const *option, const char *value, int reconfiguring); void optionQ2Dump(StoreEntry * e) const; - int send(int mtype, int id, RefCount sio, int size, int offset, off_t shm_offset); + int send(int mtype, int id, RefCount sio, int size, int offset, off_t shm_offset); void handle(diomsg * M); void unlinkDone(diomsg * M); int magic1; diff --git a/src/MemObject.h b/src/MemObject.h index 7b4f5f3158..3a778629f7 100644 --- a/src/MemObject.h +++ b/src/MemObject.h @@ -1,6 +1,6 @@ /* - * $Id: MemObject.h,v 1.11 2006/01/23 20:04:24 wessels Exp $ + * $Id: MemObject.h,v 1.12 2006/05/22 19:58:51 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -35,6 +35,7 @@ #define SQUID_MEMOBJECT_H #include "StoreIOBuffer.h" +#include "StoreIOState.h" #include "stmem.h" #include "CommRead.h" diff --git a/src/StoreClient.h b/src/StoreClient.h index 6101fe1337..8568ae698a 100644 --- a/src/StoreClient.h +++ b/src/StoreClient.h @@ -1,6 +1,6 @@ /* - * $Id: StoreClient.h,v 1.13 2004/12/27 11:04:36 serassio Exp $ + * $Id: StoreClient.h,v 1.14 2006/05/22 19:58:51 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -35,6 +35,7 @@ #define SQUID_STORECLIENT_H #include "StoreIOBuffer.h" +#include "StoreIOState.h" typedef void STCB(void *, StoreIOBuffer); /* store callback */ diff --git a/src/StoreIOState.cc b/src/StoreIOState.cc index a2bfb963d5..80cae2e15c 100644 --- a/src/StoreIOState.cc +++ b/src/StoreIOState.cc @@ -1,6 +1,6 @@ /* - * $Id: StoreIOState.cc,v 1.4 2003/08/31 12:44:30 robertc Exp $ + * $Id: StoreIOState.cc,v 1.5 2006/05/22 19:58:51 wessels Exp $ * * DEBUG: section ?? Swap Dir base object * AUTHOR: Robert Collins @@ -37,27 +37,27 @@ #include "StoreIOState.h" void * -storeIOState::operator new (size_t amount) +StoreIOState::operator new (size_t amount) { assert(0); return (void *)1; } void -storeIOState::operator delete (void *address){assert (0);} +StoreIOState::operator delete (void *address){assert (0);} -storeIOState::storeIOState() +StoreIOState::StoreIOState() { mode = O_BINARY; } off_t -storeIOState::offset() const +StoreIOState::offset() const { return offset_; } -storeIOState::~storeIOState() +StoreIOState::~StoreIOState() { debugs(20,3, "StoreIOState::~StoreIOState: " << this); diff --git a/src/StoreIOState.h b/src/StoreIOState.h index 410e1a0c7c..aa83eb57c9 100644 --- a/src/StoreIOState.h +++ b/src/StoreIOState.h @@ -1,6 +1,6 @@ /* - * $Id: StoreIOState.h,v 1.6 2003/08/04 22:14:41 robertc Exp $ + * $Id: StoreIOState.h,v 1.7 2006/05/22 19:58:51 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -34,19 +34,49 @@ #ifndef SQUID_STOREIOSTATE_H #define SQUID_STOREIOSTATE_H +/* + * STRCB is the "store read callback". STRCB functions are passed + * to storeRead(). Examples of STRCB callbacks are: + * storeClientReadBody + * storeClientReadHeader + */ +typedef void STRCB(void *their_data, const char *buf, ssize_t len); + +/* + * STFNCB is the "store file number callback." It is called when + * an underlying storage module has allocated the swap file number + * and also indicates that the swap file has been opened for reading + * or writing. STFNCB functions are passed to storeCreate() and + * storeOpen(). Examples of STFNCB callbacks are: + * storeSwapInFileNotify + * storeSwapOutFileNotify + */ +typedef void STFNCB(void *their_data, int errflag); + +/* + * STIOCB is the "store close callback" for store files. It is + * called when the store file is closed. STIOCB functions are + * passed to storeCreate() and storeOpen(). Examples of STIOCB + * callbacks are: + * storeSwapOutFileClosed + * storeSwapInFileClosed + */ +typedef void STIOCB(void *their_data, int errflag); + #include "RefCount.h" -class storeIOState : public RefCountable +class StoreIOState : public RefCountable { public: + typedef RefCount Pointer; - /* storeIOState does not get mempooled - it's children do */ + /* StoreIOState does not get mempooled - it's children do */ void *operator new (size_t amount); void operator delete (void *address); - virtual ~storeIOState(); + virtual ~StoreIOState(); - storeIOState(); + StoreIOState(); off_t offset() const; @@ -81,11 +111,10 @@ unsigned int closing: flags; }; -class StoreIOState -{ - -public: - typedef RefCount Pointer; -}; +StoreIOState::Pointer storeCreate(StoreEntry *, STFNCB *, STIOCB *, void *); +StoreIOState::Pointer storeOpen(StoreEntry *, STFNCB *, STIOCB *, void *); +SQUIDCEXTERN void storeClose(StoreIOState::Pointer); +SQUIDCEXTERN void storeRead(StoreIOState::Pointer, char *, size_t, off_t, STRCB *, void *); +SQUIDCEXTERN void storeIOWrite(StoreIOState::Pointer, char const *, size_t, off_t, FREE *); #endif /* SQUID_STOREIOSTATE_H */ diff --git a/src/fs/coss/CossSwapDir.h b/src/fs/coss/CossSwapDir.h index 4c2b398503..21e847b152 100644 --- a/src/fs/coss/CossSwapDir.h +++ b/src/fs/coss/CossSwapDir.h @@ -74,7 +74,7 @@ public: DiskIOStrategy *io; RefCount theFile; char *storeCossMemPointerFromDiskOffset(size_t offset, CossMemBuf ** mb); - void storeCossMemBufUnlock(storeIOState * e); + void storeCossMemBufUnlock(StoreIOState::Pointer); CossMemBuf *createMemBuf(size_t start, sfileno curfn, int *collision); sfileno allocate(const StoreEntry * e, int which); void startMembuf(); diff --git a/src/fs/coss/store_coss.h b/src/fs/coss/store_coss.h index 2f8c2aaa1c..2b058f5c2d 100644 --- a/src/fs/coss/store_coss.h +++ b/src/fs/coss/store_coss.h @@ -55,7 +55,7 @@ struct _cossindex /* Per-storeiostate info */ -class CossState : public storeIOState +class CossState : public StoreIOState { public: diff --git a/src/fs/ufs/store_io_ufs.cc b/src/fs/ufs/store_io_ufs.cc index 17219748f1..ca001f9df6 100644 --- a/src/fs/ufs/store_io_ufs.cc +++ b/src/fs/ufs/store_io_ufs.cc @@ -1,6 +1,6 @@ /* - * $Id: store_io_ufs.cc,v 1.29 2006/05/19 16:49:13 wessels Exp $ + * $Id: store_io_ufs.cc,v 1.30 2006/05/22 19:58:53 wessels Exp $ * * DEBUG: section 79 Storage Manager UFS Interface * AUTHOR: Duane Wessels @@ -299,7 +299,7 @@ UFSStoreState::doCallback(int errflag) void *cbdata; if (cbdataReferenceValidDone(callback_data, &cbdata) && theCallback) - theCallback(cbdata, errflag, this); + theCallback(cbdata, errflag); /* We are finished with the file as this is on close or error only.*/ /* This must be the last line, as theFile may be the only object holding diff --git a/src/fs/ufs/ufscommon.h b/src/fs/ufs/ufscommon.h index a400fee258..161253001a 100644 --- a/src/fs/ufs/ufscommon.h +++ b/src/fs/ufs/ufscommon.h @@ -1,6 +1,6 @@ /* - * $Id: ufscommon.h,v 1.3 2005/02/05 22:02:32 serassio Exp $ + * $Id: ufscommon.h,v 1.4 2006/05/22 19:58:53 wessels Exp $ * * SQUID Web Proxy Cache http://www.squid-cache.org/ * ---------------------------------------------------------- @@ -196,7 +196,7 @@ protected: class ReadRequest; -class UFSStoreState : public storeIOState, public IORequestor +class UFSStoreState : public StoreIOState, public IORequestor { public: diff --git a/src/protos.h b/src/protos.h index 078ee451ce..371f11423d 100644 --- a/src/protos.h +++ b/src/protos.h @@ -1,6 +1,6 @@ /* - * $Id: protos.h,v 1.531 2006/05/10 21:04:24 hno Exp $ + * $Id: protos.h,v 1.532 2006/05/22 19:58:51 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -577,14 +577,6 @@ SQUIDCEXTERN void memConfigure(void); /* repl_modules.c */ SQUIDCEXTERN void storeReplSetup(void); -/* store_io.c */ -#include "StoreIOState.h" -extern StoreIOState::Pointer storeCreate(StoreEntry *, STFNCB *, STIOCB *, void *); -extern StoreIOState::Pointer storeOpen(StoreEntry *, STFNCB *, STIOCB *, void *); -SQUIDCEXTERN void storeClose(StoreIOState::Pointer); -SQUIDCEXTERN void storeRead(StoreIOState::Pointer, char *, size_t, off_t, STRCB *, void *); -SQUIDCEXTERN void storeIOWrite(StoreIOState::Pointer, char const *, size_t, off_t, FREE *); - /* * store_log.c */ diff --git a/src/store_io.cc b/src/store_io.cc index 4d0479ca52..21a9f7c612 100644 --- a/src/store_io.cc +++ b/src/store_io.cc @@ -28,7 +28,7 @@ OBJH storeIOStats; * to select different polices depending on object size or type. */ StoreIOState::Pointer -storeCreate(StoreEntry * e, STIOCB * file_callback, STIOCB * close_callback, void *callback_data) +storeCreate(StoreEntry * e, STFNCB * file_callback, STIOCB * close_callback, void *callback_data) { assert (e); ssize_t objsize; diff --git a/src/store_swapin.cc b/src/store_swapin.cc index 5e30c2e472..b780c9086e 100644 --- a/src/store_swapin.cc +++ b/src/store_swapin.cc @@ -1,6 +1,6 @@ /* - * $Id: store_swapin.cc,v 1.36 2003/10/20 12:34:01 robertc Exp $ + * $Id: store_swapin.cc,v 1.37 2006/05/22 19:58:51 wessels Exp $ * * DEBUG: section 20 Storage Manager Swapin Functions * AUTHOR: Duane Wessels @@ -72,13 +72,12 @@ storeSwapInStart(store_client * sc) } static void -storeSwapInFileClosed(void *data, int errflag, storeIOState * sio) +storeSwapInFileClosed(void *data, int errflag) { store_client *sc = (store_client *)data; debug(20, 3) ("storeSwapInFileClosed: sio=%p, errflag=%d\n", - sio, errflag); + sc->swapin_sio.getRaw(), errflag); sc->swapin_sio = NULL; - /* why this assert */ if (sc->_callback.pending()) { assert (errflag <= 0); @@ -89,13 +88,13 @@ storeSwapInFileClosed(void *data, int errflag, storeIOState * sio) } static void -storeSwapInFileNotify(void *data, int errflag, storeIOState * sio) +storeSwapInFileNotify(void *data, int errflag) { store_client *sc = (store_client *)data; StoreEntry *e = sc->entry; - debug(1, 3) ("storeSwapInFileNotify: changing %d/%d to %d/%d\n", e->swap_filen, e->swap_dirn, sio->swap_filen, sio->swap_dirn); + debug(1, 3) ("storeSwapInFileNotify: changing %d/%d to %d/%d\n", e->swap_filen, e->swap_dirn, sc->swapin_sio->swap_filen, sc->swapin_sio->swap_dirn); - e->swap_filen = sio->swap_filen; - e->swap_dirn = sio->swap_dirn; + e->swap_filen = sc->swapin_sio->swap_filen; + e->swap_dirn = sc->swapin_sio->swap_dirn; } diff --git a/src/store_swapout.cc b/src/store_swapout.cc index e4d2c626d0..4d3089890b 100644 --- a/src/store_swapout.cc +++ b/src/store_swapout.cc @@ -1,6 +1,6 @@ /* - * $Id: store_swapout.cc,v 1.103 2006/05/19 17:19:10 wessels Exp $ + * $Id: store_swapout.cc,v 1.104 2006/05/22 19:58:51 wessels Exp $ * * DEBUG: section 20 Storage Manager Swapout Functions * AUTHOR: Duane Wessels @@ -43,7 +43,7 @@ static void storeSwapOutStart(StoreEntry * e); static STIOCB storeSwapOutFileClosed; -static STIOCB storeSwapOutFileNotify; +static STFNCB storeSwapOutFileNotify; /* start swapping object to disk */ static void @@ -100,14 +100,14 @@ storeSwapOutStart(StoreEntry * e) } static void -storeSwapOutFileNotify(void *data, int errflag, storeIOState * sio) +storeSwapOutFileNotify(void *data, int errflag) { generic_cbdata *c = (generic_cbdata *)data; StoreEntry *e = (StoreEntry *)c->data; MemObject *mem = e->mem_obj; assert(e->swap_status == SWAPOUT_WRITING); assert(mem); - assert(mem->swapout.sio == sio); + assert(mem->swapout.sio != NULL); assert(errflag == 0); e->swap_filen = mem->swapout.sio->swap_filen; e->swap_dirn = mem->swapout.sio->swap_dirn; @@ -316,7 +316,7 @@ storeSwapOutFileClose(StoreEntry * e) } static void -storeSwapOutFileClosed(void *data, int errflag, storeIOState * sio) +storeSwapOutFileClosed(void *data, int errflag) { generic_cbdata *c = (generic_cbdata *)data; StoreEntry *e = (StoreEntry *)c->data; diff --git a/src/tests/TestSwapDir.h b/src/tests/TestSwapDir.h index db28d95b95..0843c99127 100644 --- a/src/tests/TestSwapDir.h +++ b/src/tests/TestSwapDir.h @@ -18,10 +18,10 @@ public: virtual void reconfigure(int, char*); virtual void init(); virtual int canStore(const StoreEntry&) const; - virtual RefCount createStoreIO(StoreEntry&, void - (*)(void*, int, storeIOState*), void (*)(void*, int, storeIOState*), void*); - virtual RefCount openStoreIO(StoreEntry&, void - (*)(void*, int, storeIOState*), void (*)(void*, int, storeIOState*), void*); + virtual RefCount createStoreIO(StoreEntry&, void + (*)(void*, int), void (*)(void*, int), void*); + virtual RefCount openStoreIO(StoreEntry&, void + (*)(void*, int), void (*)(void*, int), void*); virtual void parse(int, char*); virtual StoreSearch *search(String, HttpRequest *); }; diff --git a/src/typedefs.h b/src/typedefs.h index 6281ff1927..9f8c5d07c2 100644 --- a/src/typedefs.h +++ b/src/typedefs.h @@ -1,6 +1,6 @@ /* - * $Id: typedefs.h,v 1.185 2006/05/19 23:10:21 wessels Exp $ + * $Id: typedefs.h,v 1.186 2006/05/22 19:58:51 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -215,8 +215,6 @@ typedef struct _helper_stateful_server helper_stateful_server; typedef struct _generic_cbdata generic_cbdata; -class storeIOState; - typedef struct _link_list link_list; typedef struct _Logfile Logfile; @@ -274,10 +272,6 @@ typedef void UH(void *data, wordlist *); typedef int READ_HANDLER(int, char *, int); typedef int WRITE_HANDLER(int, const char *, int); -typedef void STIOCB(void *their_data, int errflag, storeIOState *); -typedef void STFNCB(void *their_data, int errflag, storeIOState *); -typedef void STRCB(void *their_data, const char *buf, ssize_t len); - typedef int QS(const void *, const void *); /* qsort */ typedef void STABH(void *); typedef void ERCB(int fd, void *, size_t); -- 2.47.3