From: Amos Jeffries Date: Mon, 5 Dec 2022 15:52:26 +0000 (+0000) Subject: Drop CBDATA debugging (#1199) X-Git-Tag: SQUID_6_0_1~65 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=bcd05a84ce543a50c354435e58ad6850f29e59e8;p=thirdparty%2Fsquid.git Drop CBDATA debugging (#1199) This was occasionally (but still rare) useful when CBDATA was the only method of passing callback parameters. Much of that has now been replaced with AsyncJob/AsyncCall logic. Making this debug much less useful. Remove the --enable-debug-cbdata build option and two cache manager reports only enabled when that option is built. Greatly reducing the cbdata logic complexity and removing one of its five APIs permutations entirely. --- diff --git a/configure.ac b/configure.ac index 518fa9b1ab..313373cf33 100644 --- a/configure.ac +++ b/configure.ac @@ -352,16 +352,6 @@ AC_ARG_ENABLE(optimizations, ]) ]) -AC_ARG_ENABLE(debug-cbdata, - AS_HELP_STRING([--enable-debug-cbdata], - [Provide some debug information in cbdata]), [ - SQUID_YESNO([$enableval],[--enable-debug-cbdata]) -]) -SQUID_DEFINE_BOOL(USE_CBDATA_DEBUG,${enable_debug_cbdata:=no}, - [Enable support for cbdata debug information]) -AC_MSG_NOTICE([cbdata debugging enabled: $enable_debug_cbdata]) - - dnl Nasty hack to get autoconf 2.64 on Linux to run. dnl all other uses of RUN_IFELSE are wrapped inside CACHE_CHECK which breaks on 2.64 AC_RUN_IFELSE([AC_LANG_SOURCE([[ int main(int argc, char **argv) { return 0; } ]])],[],[],[:]) diff --git a/doc/release-notes/release-6.sgml b/doc/release-notes/release-6.sgml index e015ed3548..78267b31da 100644 --- a/doc/release-notes/release-6.sgml +++ b/doc/release-notes/release-6.sgml @@ -126,6 +126,10 @@ This section gives an account of those changes in three categories:

This feature has been unreliable for many years. Other tools such as oprofile provide better tracking and should be used instead. + --enable-debug-cbdata +

This feature has been of limited use since AsyncCalls feature + took over much of the CBDATA functionality. + --enable-kill-parent-hack

This feature has been deprecated for years. Other features such as --foreground command line argument should be used instead. diff --git a/src/base/CbcPointer.h b/src/base/CbcPointer.h index 283a2a58ca..967318aeda 100644 --- a/src/base/CbcPointer.h +++ b/src/base/CbcPointer.h @@ -143,9 +143,6 @@ template void CbcPointer::clear() { -#if USE_CBDATA_DEBUG - debugs(45, 3, "cbc=" << (void*)cbc << ", lock=" << (void*)lock); -#endif cbdataReferenceDone(lock); // lock may be nil before and will be nil after cbc = nullptr; } diff --git a/src/cbdata.cc b/src/cbdata.cc index 9a9b892d3f..9d52bae935 100644 --- a/src/cbdata.cc +++ b/src/cbdata.cc @@ -18,34 +18,11 @@ #include #include -#if USE_CBDATA_DEBUG -#include -#include -#endif - #if WITH_VALGRIND #include #endif static int cbdataCount = 0; -#if USE_CBDATA_DEBUG -dlink_list cbdataEntries; -#endif - -#if USE_CBDATA_DEBUG - -class CBDataCall -{ - -public: - CBDataCall (char const *callLabel, char const *aFile, int aLine) : label(callLabel), file(aFile), line(aLine) {} - - char const *label; - char const *file; - int line; -}; - -#endif /** * Manage a set of registered callback data pointers. @@ -73,18 +50,10 @@ public: /* TODO: examine making cbdata templated on this - so we get type * safe access to data - RBC 20030902 */ public: -#if USE_CBDATA_DEBUG - - void dump(StoreEntry *)const; -#endif cbdata() : valid(0), locks(0), type(CBDATA_UNKNOWN), -#if USE_CBDATA_DEBUG - file(nullptr), - line(0), -#endif cookie(0), data(nullptr) {} @@ -95,20 +64,6 @@ public: int valid; int32_t locks; cbdata_type type; -#if USE_CBDATA_DEBUG - - void addHistory(char const *label, char const *aFile, int aLine) { - if (calls.size() > 1000) - return; - - calls.push_back(new CBDataCall(label, aFile, aLine)); - } - - dlink_node link; - const char *file; - int line; - std::vector calls; // used as a stack with random access operator -#endif /* cookie used while debugging */ long cookie; @@ -126,11 +81,6 @@ static_assert(std::is_standard_layout::value, "the behavior of offsetof( const long cbdata::Cookie((long)0xDEADBEEF); -static OBJH cbdataDump; -#if USE_CBDATA_DEBUG -static OBJH cbdataDumpHistory; -#endif - struct CBDataIndex { Mem::Allocator *pool; } @@ -144,15 +94,6 @@ static std::map cbdata_htable; cbdata::~cbdata() { -#if USE_CBDATA_DEBUG - - while (!calls.empty()) { - delete calls.back(); - calls.pop_back(); - } - -#endif - #if WITH_VALGRIND void *p = data; #else @@ -171,9 +112,14 @@ cbdata::FromUserData(const void *p) { #endif } -static void -cbdataInternalInitType(cbdata_type type, const char *name, int size) +cbdata_type +cbdataInternalAddType(cbdata_type type, const char *name, int size) { + if (type) + return type; + + type = (cbdata_type)(cbdata_types + 1); + char *label; assert (type == cbdata_types + 1); @@ -190,37 +136,12 @@ cbdataInternalInitType(cbdata_type type, const char *name, int size) #endif cbdata_index[type].pool = memPoolCreate(label, size); -} - -cbdata_type -cbdataInternalAddType(cbdata_type type, const char *name, int size) -{ - if (type) - return type; - - type = (cbdata_type)(cbdata_types + 1); - - cbdataInternalInitType(type, name, size); return type; } -void -cbdataRegisterWithCacheManager(void) -{ - Mgr::RegisterAction("cbdata", - "Callback Data Registry Contents", - cbdataDump, 0, 1); -#if USE_CBDATA_DEBUG - - Mgr::RegisterAction("cbdatahistory", - "Detailed call history for all current cbdata contents", - cbdataDumpHistory, 0, 1); -#endif -} - void * -cbdataInternalAlloc(cbdata_type type, const char *file, int line) +cbdataInternalAlloc(cbdata_type type) { cbdata *c; void *p; @@ -243,25 +164,12 @@ cbdataInternalAlloc(cbdata_type type, const char *file, int line) c->locks = 0; c->cookie = (long) c ^ cbdata::Cookie; ++cbdataCount; -#if USE_CBDATA_DEBUG - - c->file = file; - c->line = line; - c->calls = std::vector (); - c->addHistory("Alloc", file, line); - dlinkAdd(c, &c->link, &cbdataEntries); - debugs(45, 3, "Allocating " << p << " " << file << ":" << line); -#else - (void)file; - (void)line; debugs(45, 9, "Allocating " << p); -#endif - return p; } static void -cbdataRealFree(cbdata *c, const char *file, const int line) +cbdataRealFree(cbdata *c) { #if WITH_VALGRIND void *p = c->data; @@ -270,14 +178,7 @@ cbdataRealFree(cbdata *c, const char *file, const int line) #endif --cbdataCount; -#if USE_CBDATA_DEBUG - debugs(45, 3, "Freeing " << p << ' ' << file << ':' << line); - dlinkDelete(&c->link, &cbdataEntries); -#else debugs(45, 9, "Freeing " << p); - (void)file; - (void)line; -#endif #if WITH_VALGRIND cbdata_htable.erase(p); @@ -298,50 +199,32 @@ cbdataRealFree(cbdata *c, const char *file, const int line) } void * -cbdataInternalFree(void *p, const char *file, int line) +cbdataInternalFree(void *p) { auto *c = cbdata::FromUserData(p); -#if USE_CBDATA_DEBUG - debugs(45, 3, p << " " << file << ":" << line); -#else - debugs(45, 9, p); -#endif c->check(__LINE__); assert(c->valid); c->valid = 0; -#if USE_CBDATA_DEBUG - - c->addHistory("Free", file, line); -#endif if (c->locks) { debugs(45, 9, p << " has " << c->locks << " locks, not freeing"); return nullptr; } - cbdataRealFree(c, file, line); + cbdataRealFree(c); return nullptr; } void -#if USE_CBDATA_DEBUG -cbdataInternalLockDbg(const void *p, const char *file, int line) -#else cbdataInternalLock(const void *p) -#endif { if (p == nullptr) return; auto *c = cbdata::FromUserData(p); -#if USE_CBDATA_DEBUG - debugs(45, 3, p << "=" << (c ? c->locks + 1 : -1) << " " << file << ":" << line); - c->addHistory("Reference", file, line); -#else debugs(45, 9, p << "=" << (c ? c->locks + 1 : -1)); -#endif c->check(__LINE__); @@ -351,23 +234,14 @@ cbdataInternalLock(const void *p) } void -#if USE_CBDATA_DEBUG -cbdataInternalUnlockDbg(const void *p, const char *file, int line) -#else cbdataInternalUnlock(const void *p) -#endif { if (p == nullptr) return; auto *c = cbdata::FromUserData(p); -#if USE_CBDATA_DEBUG - debugs(45, 3, p << "=" << (c ? c->locks - 1 : -1) << " " << file << ":" << line); - c->addHistory("Dereference", file, line); -#else debugs(45, 9, p << "=" << (c ? c->locks - 1 : -1)); -#endif c->check(__LINE__); @@ -380,18 +254,10 @@ cbdataInternalUnlock(const void *p) if (c->locks) return; - if (c->valid) { -#if USE_CBDATA_DEBUG - debugs(45, 3, "CBDATA valid with no references ... cbdata=" << p << " " << file << ":" << line); -#endif + if (c->valid) return; - } -#if USE_CBDATA_DEBUG - cbdataRealFree(c, file, line); -#else - cbdataRealFree(c, NULL, 0); -#endif + cbdataRealFree(c); } int @@ -412,22 +278,13 @@ cbdataReferenceValid(const void *p) } int -#if USE_CBDATA_DEBUG -cbdataInternalReferenceDoneValidDbg(void **pp, void **tp, const char *file, int line) -#else cbdataInternalReferenceDoneValid(void **pp, void **tp) -#endif { void *p = (void *) *pp; int valid = cbdataReferenceValid(p); *pp = nullptr; -#if USE_CBDATA_DEBUG - - cbdataInternalUnlockDbg(p, file, line); -#else cbdataInternalUnlock(p); -#endif if (valid) { *tp = p; @@ -438,62 +295,6 @@ cbdataInternalReferenceDoneValid(void **pp, void **tp) } } -#if USE_CBDATA_DEBUG -void -cbdata::dump(StoreEntry *sentry) const -{ -#if WITH_VALGRIND - void *p = data; -#else - void *p = (void *)&data; -#endif - storeAppendPrintf(sentry, "%c%p\t%d\t%d\t%20s:%-5d\n", valid ? ' ' : - '!', p, type, locks, file, line); -} - -struct CBDataDumper : public unary_function { - CBDataDumper(StoreEntry *anEntry):where(anEntry) {} - - void operator()(cbdata const &x) { - x.dump(where); - } - - StoreEntry *where; -}; - -#endif - -static void -cbdataDump(StoreEntry * sentry) -{ - storeAppendPrintf(sentry, "%d cbdata entries\n", cbdataCount); -#if USE_CBDATA_DEBUG - - storeAppendPrintf(sentry, "Pointer\tType\tLocks\tAllocated by\n"); - CBDataDumper dumper(sentry); - for_each (cbdataEntries, dumper); - storeAppendPrintf(sentry, "\n"); - storeAppendPrintf(sentry, "types\tsize\tallocated\ttotal\n"); - - for (int i = 1; i < cbdata_types; ++i) { - if (const auto pool = cbdata_index[i].pool) { -#if WITH_VALGRIND - int obj_size = pool->objectSize(); -#else - int obj_size = pool->objectSize() - offsetof(cbdata, data); -#endif - storeAppendPrintf(sentry, "%s\t%d\t%ld\t%ld\n", pool->objectType() + 7, obj_size, (long int)pool->getMeter().inuse.currentLevel(), (long int)obj_size * pool->getMeter().inuse.currentLevel()); - } - } - -#else - storeAppendPrintf(sentry, "detailed allocation information only available when compiled with --enable-debug-cbdata\n"); - -#endif - - storeAppendPrintf(sentry, "\nsee also \"Memory utilization\" for detailed per type statistics\n"); -} - CallbackData & CallbackData::operator =(const CallbackData &other) { @@ -507,41 +308,3 @@ CallbackData::operator =(const CallbackData &other) CBDATA_CLASS_INIT(generic_cbdata); -#if USE_CBDATA_DEBUG - -struct CBDataCallDumper : public unary_function { - CBDataCallDumper (StoreEntry *anEntry):where(anEntry) {} - - void operator()(CBDataCall * const &x) { - storeAppendPrintf(where, "%s\t%s\t%d\n", x->label, x->file, x->line); - } - - StoreEntry *where; -}; - -struct CBDataHistoryDumper : public CBDataDumper { - CBDataHistoryDumper(StoreEntry *anEntry):CBDataDumper(anEntry),where(anEntry), callDumper(anEntry) {} - - void operator()(cbdata const &x) { - CBDataDumper::operator()(x); - storeAppendPrintf(where, "\n"); - storeAppendPrintf(where, "Action\tFile\tLine\n"); - std::for_each (x.calls.begin(), x.calls.end(), callDumper); - storeAppendPrintf(where, "\n"); - } - - StoreEntry *where; - CBDataCallDumper callDumper; -}; - -void -cbdataDumpHistory(StoreEntry *sentry) -{ - storeAppendPrintf(sentry, "%d cbdata entries\n", cbdataCount); - storeAppendPrintf(sentry, "Pointer\tType\tLocks\tAllocated by\n"); - CBDataHistoryDumper dumper(sentry); - for_each (cbdataEntries, dumper); -} - -#endif - diff --git a/src/cbdata.h b/src/cbdata.h index 3c3d5d92ec..b76f4199f7 100644 --- a/src/cbdata.h +++ b/src/cbdata.h @@ -195,18 +195,12 @@ typedef int cbdata_type; static const cbdata_type CBDATA_UNKNOWN = 0; -/** - * Create a run-time registration of CBDATA component with - * the Squid cachemgr - */ -void cbdataRegisterWithCacheManager(void); - /** * Allocates a new entry of a registered CBDATA type. * * \note For internal CBDATA use only. */ -void *cbdataInternalAlloc(cbdata_type type, const char *, int); +void *cbdataInternalAlloc(cbdata_type type); /** * Frees a entry allocated by cbdataInternalAlloc(). @@ -221,19 +215,8 @@ void *cbdataInternalAlloc(cbdata_type type, const char *, int); * * \note For internal CBDATA use only. */ -void *cbdataInternalFree(void *p, const char *, int); - -#if USE_CBDATA_DEBUG -void cbdataInternalLockDbg(const void *p, const char *, int); -#define cbdataInternalLock(a) cbdataInternalLockDbg(a,__FILE__,__LINE__) +void *cbdataInternalFree(void *p); -void cbdataInternalUnlockDbg(const void *p, const char *, int); -#define cbdataInternalUnlock(a) cbdataInternalUnlockDbg(a,__FILE__,__LINE__) - -int cbdataInternalReferenceDoneValidDbg(void **p, void **tp, const char *, int); -#define cbdataReferenceValidDone(var, ptr) cbdataInternalReferenceDoneValidDbg((void **)&(var), (ptr), __FILE__,__LINE__) - -#else void cbdataInternalLock(const void *p); void cbdataInternalUnlock(const void *p); @@ -255,8 +238,6 @@ void cbdataInternalUnlock(const void *p); int cbdataInternalReferenceDoneValid(void **p, void **tp); #define cbdataReferenceValidDone(var, ptr) cbdataInternalReferenceDoneValid((void **)&(var), (ptr)) -#endif /* !CBDATA_DEBUG */ - /** * \param p A cbdata entry reference pointer. * @@ -278,10 +259,10 @@ cbdata_type cbdataInternalAddType(cbdata_type type, const char *label, int size) void *operator new(size_t size) { \ assert(size == sizeof(type)); \ if (!CBDATA_##type) CBDATA_##type = cbdataInternalAddType(CBDATA_##type, #type, sizeof(type)); \ - return (type *)cbdataInternalAlloc(CBDATA_##type,__FILE__,__LINE__); \ + return (type *)cbdataInternalAlloc(CBDATA_##type); \ } \ void operator delete (void *address) { \ - if (address) cbdataInternalFree(address,__FILE__,__LINE__); \ + if (address) cbdataInternalFree(address); \ } \ void *toCbdata() methodSpecifiers { return this; } \ private: \ diff --git a/src/main.cc b/src/main.cc index 4037be325f..b912c11ef0 100644 --- a/src/main.cc +++ b/src/main.cc @@ -1201,9 +1201,6 @@ mainInitialize(void) #endif FwdState::initModule(); - /* register the modules in the cache manager menus */ - - cbdataRegisterWithCacheManager(); SBufStatsAction::RegisterWithCacheManager(); /* These use separate calls so that the comm loops can eventually diff --git a/src/tests/stub_cbdata.cc b/src/tests/stub_cbdata.cc index 3f8169b981..69d3f49323 100644 --- a/src/tests/stub_cbdata.cc +++ b/src/tests/stub_cbdata.cc @@ -7,29 +7,16 @@ */ #include "squid.h" -#include "cbdata.h" #define STUB_API "cbdata.cc" #include "tests/STUB.h" -void cbdataRegisterWithCacheManager(void) STUB -void *cbdataInternalAlloc(cbdata_type, const char *, int sz) { - return xcalloc(1, sz); -} -void *cbdataInternalFree(void *p, const char *, int) { - xfree(p); - return nullptr; -} -#if USE_CBDATA_DEBUG -void cbdataInternalLockDbg(const void *, const char *, int) STUB -void cbdataInternalUnlockDbg(const void *, const char *, int) STUB -int cbdataInternalReferenceDoneValidDbg(void **, void **, const char *, int) STUB_RETVAL(0) -#else +#include "cbdata.h" +void *cbdataInternalAlloc(cbdata_type) STUB_RETVAL(nullptr) +void *cbdataInternalFree(void *) STUB_RETVAL(nullptr) void cbdataInternalLock(const void *) STUB void cbdataInternalUnlock(const void *) STUB int cbdataInternalReferenceDoneValid(void **, void **) STUB_RETVAL(0) -#endif - int cbdataReferenceValid(const void *) STUB_RETVAL(0) cbdata_type cbdataInternalAddType(cbdata_type, const char *, int) STUB_RETVAL(CBDATA_UNKNOWN) diff --git a/test-suite/buildtests/layer-01-minimal.opts b/test-suite/buildtests/layer-01-minimal.opts index 5720022e34..bb88bda275 100644 --- a/test-suite/buildtests/layer-01-minimal.opts +++ b/test-suite/buildtests/layer-01-minimal.opts @@ -40,7 +40,6 @@ DISTCHECK_CONFIGURE_FLAGS=" \ --disable-build-info \ --disable-shared \ --disable-gnuregex \ - --disable-debug-cbdata \ --disable-xmalloc-statistics \ --disable-async-io \ --disable-storeio \ diff --git a/test-suite/buildtests/layer-02-maximus.opts b/test-suite/buildtests/layer-02-maximus.opts index 3ddd173371..8ce7d0908f 100644 --- a/test-suite/buildtests/layer-02-maximus.opts +++ b/test-suite/buildtests/layer-02-maximus.opts @@ -60,7 +60,6 @@ DISTCHECK_CONFIGURE_FLAGS=" \ --enable-shared \ --enable-gnuregex \ --enable-optimizations \ - --enable-debug-cbdata \ --enable-xmalloc-statistics \ --enable-async-io \ --enable-storeio \ diff --git a/test-suite/buildtests/layer-04-noauth-everything.opts b/test-suite/buildtests/layer-04-noauth-everything.opts index 552c210f55..eed5a37862 100644 --- a/test-suite/buildtests/layer-04-noauth-everything.opts +++ b/test-suite/buildtests/layer-04-noauth-everything.opts @@ -60,7 +60,6 @@ DISTCHECK_CONFIGURE_FLAGS=" \ --enable-gnuregex \ --enable-optimizations \ --enable-inline \ - --enable-debug-cbdata \ --enable-xmalloc-statistics \ --enable-async-io \ --enable-storeio \