Avoid startup/shutdown crashes [by avoiding static non-POD globals].

Squid crashes on startup when the parent process exit()s after fork()ing
the kid process. Squid may also crash on shutdown after exiting main().

In both cases, the crashes are build- and environment-specific. Many
environments show no problems at all. Even disabling compiler
optimizations may prevent crashes. When crashes do happen, their
symptoms (e.g., backtrace) point to problems during destruction of
global objects, but the details point to innocent objects (e.g., PortCfg
or SSL_CTX).

In some environments, the following malloc error is printed on console
before the crash: "corrupted double-linked list".

This change replaces two StatHist globals used for SBuf statistics
collection with always-available singletons. The replaced globals could
be destructed before the last SBuf object using them, leading to memory
corruption (that would eventually crash Squid).

There are probably more such globals.

diff --git a/src/SBufDetailedStats.cc b/src/SBufDetailedStats.cc
index d5f7471422a9344c318698c1e9c93d20127f2121..40c6ed125d9e1ccefc4023c15621442e9879c6d1 100644 (file)
--- a/src/SBufDetailedStats.cc
+++ b/src/SBufDetailedStats.cc
@@ -15,42 +15,36 @@
  * external dependencies to the SBuf code
  */
 
-static StatHist sbufDestructTimeStats;
-static StatHist memblobDestructTimeStats;
-
-namespace SBufDetailedStatsHistInitializer
-{
-// run the post-instantiation initialization methods for StatHist objects
-struct Initializer {
-    Initializer() {
-        sbufDestructTimeStats.logInit(100,30.0,128000.0);
-        memblobDestructTimeStats.logInit(100,30.0,128000.0);
-    }
-};
-Initializer initializer;
+static StatHist *
+newStatHist() {
+    StatHist *stats = new StatHist;
+    stats->logInit(100, 30.0, 128000.0);
+    return stats;
 }
 
-void
-recordSBufSizeAtDestruct(SBuf::size_type sz)
+StatHist &
+collectSBufDestructTimeStats()
 {
-    sbufDestructTimeStats.count(static_cast<double>(sz));
+    static StatHist *stats = newStatHist();
+    return *stats;
 }
 
-const StatHist *
-collectSBufDestructTimeStats()
+StatHist &
+collectMemBlobDestructTimeStats()
 {
-    return &sbufDestructTimeStats;
+    static StatHist *stats = newStatHist();
+    return *stats;
 }
 
 void
-recordMemBlobSizeAtDestruct(SBuf::size_type sz)
+recordSBufSizeAtDestruct(SBuf::size_type sz)
 {
-    memblobDestructTimeStats.count(static_cast<double>(sz));
+    collectSBufDestructTimeStats().count(static_cast<double>(sz));
 }
 
-const StatHist *
-collectMemBlobDestructTimeStats()
+void
+recordMemBlobSizeAtDestruct(SBuf::size_type sz)
 {
-    return &memblobDestructTimeStats;
+    collectMemBlobDestructTimeStats().count(static_cast<double>(sz));
 }
 

diff --git a/src/SBufDetailedStats.h b/src/SBufDetailedStats.h
index 86761c1cb948d095ab0e7ce28a2a672385e3ebcf..8cc842f1604f65cc30a23ed0f45e1cbd4c0c8f0b 100644 (file)
--- a/src/SBufDetailedStats.h
+++ b/src/SBufDetailedStats.h
@@ -16,20 +16,14 @@ class StatHist;
 /// Record the size a SBuf had when it was destructed
 void recordSBufSizeAtDestruct(SBuf::size_type sz);
 
-/** Collect the SBuf size-at-destruct-time histogram
- *
- * \note the returned StatHist object must not be freed
- */
-const StatHist * collectSBufDestructTimeStats();
+/// the SBuf size-at-destruct-time histogram
+StatHist &collectSBufDestructTimeStats();
 
 /// Record the size a MemBlob had when it was destructed
 void recordMemBlobSizeAtDestruct(MemBlob::size_type sz);
 
-/** Collect the MemBlob size-at-destruct-time histogram
- *
- * \note the returned StatHist object must not be freed
- */
-const StatHist * collectMemBlobDestructTimeStats();
+/// the MemBlob size-at-destruct-time histogram
+StatHist &collectMemBlobDestructTimeStats();
 
 #endif /* SQUID_SBUFDETAILEDSTATS_H */
 

diff --git a/src/SBufStatsAction.cc b/src/SBufStatsAction.cc
index 1e58ee960dae66f0276c93f18376d6bb625037ed..aaa2a29a09bdc8ac35a00ff2a760ea6b51384161 100644 (file)
--- a/src/SBufStatsAction.cc
+++ b/src/SBufStatsAction.cc
@@ -38,8 +38,8 @@ SBufStatsAction::collect()
 {
     sbdata = SBuf::GetStats();
     mbdata = MemBlob::GetStats();
-    sbsizesatdestruct = *collectSBufDestructTimeStats();
-    mbsizesatdestruct = *collectMemBlobDestructTimeStats();
+    sbsizesatdestruct = collectSBufDestructTimeStats();
+    mbsizesatdestruct = collectMemBlobDestructTimeStats();
 }
 
 static void