From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Mon, 11 Apr 2016 15:14:58 +0000 (-0600)
Subject: Avoid startup/shutdown crashes [by avoiding static non-POD globals].
X-Git-Tag: SQUID_4_0_9~14
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b3c7741cc75ba522b6fe86e64d44688089293c89;p=thirdparty%2Fsquid.git

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/SBufStatsAction.cc b/src/SBufStatsAction.cc
index 296c9e390f..d97fb03ce9 100644
--- 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
diff --git a/src/sbuf/DetailedStats.cc b/src/sbuf/DetailedStats.cc
index 3d1a181b1b..8ffbf89de1 100644
--- a/src/sbuf/DetailedStats.cc
+++ b/src/sbuf/DetailedStats.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/sbuf/DetailedStats.h b/src/sbuf/DetailedStats.h
index f5bc27f819..97501ea5d5 100644
--- a/src/sbuf/DetailedStats.h
+++ b/src/sbuf/DetailedStats.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 */