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 518fa9b1abf23cf59bf6a0f85bd5cc0347391243..313373cf33329e0b8202c52b3fdc2489102504c5 100644 (file)
--- 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 e015ed354858618d9a9036c310c4ea062acafc3e..78267b31daa6cc64d6e211a7b2a185780da8aad4 100644 (file)
--- 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:
        <p>This feature has been unreliable for many years. Other tools such as
           oprofile provide better tracking and should be used instead.
 
+       <tag>--enable-debug-cbdata</tag>
+       <p>This feature has been of limited use since AsyncCalls feature
+          took over much of the CBDATA functionality.
+
        <tag>--enable-kill-parent-hack</tag>
        <p>This feature has been deprecated for years. Other features such as
           <em>--foreground</em> command line argument should be used instead.

diff --git a/src/base/CbcPointer.h b/src/base/CbcPointer.h
index 283a2a58caab4c708ec08f5a10877df2bb27bc9b..967318aeda63794f31ad5bbeff7c410d40844812 100644 (file)
--- a/src/base/CbcPointer.h
+++ b/src/base/CbcPointer.h
@@ -143,9 +143,6 @@ template<class Cbc>
 void
 CbcPointer<Cbc>::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 9a9b892d3f883711dc93c5f4672f0713a8a79f54..9d52bae9352999f8c716bfc9330212309c545812 100644 (file)
--- a/src/cbdata.cc
+++ b/src/cbdata.cc
@@ -18,34 +18,11 @@
 #include <climits>
 #include <cstddef>
 
-#if USE_CBDATA_DEBUG
-#include <algorithm>
-#include <vector>
-#endif
-
 #if WITH_VALGRIND
 #include <map>
 #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<CBDataCall*> 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<cbdata>::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<const void *, cbdata *> 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<CBDataCall *> ();
-    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<cbdata, void> {
-    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<CBDataCall, void> {
-    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 3c3d5d92ec133cd24e26db6a76fa1b608f303d16..b76f4199f74c57b0c115fdf95a9d083fe6a0c790 100644 (file)
--- 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 4037be325f1eef825fcc1b664ab9fc1dde2d6003..b912c11ef051d88df58e00ccb9a7370ad2536163 100644 (file)
--- 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 3f8169b9819f9f513035234cb5131e56a811cff8..69d3f493237019b6d9972901cffd962ad5b57463 100644 (file)
--- 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 5720022e34c3d90848f58f5187f8cffb5b8bf59d..bb88bda2751f56cb5befdf47d0cb179347943d0d 100644 (file)
--- 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 3ddd17337137b996976293fe2649118faa4eb0dc..8ce7d0908f83ca4994ecd54bc1732563e0f4eaa8 100644 (file)
--- 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 552c210f55f85c1642edf93c68a507cda39bc11f..eed5a37862f22c4d47f6ec9b2b4bab169af59ca0 100644 (file)
--- 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 \