QPID-3147: Misconfigured tracing/logging can lead to hung threads in logging stack

The hang was caused by re-entrant attempts to initialize the Logger
singleton when an exception was thrown during logger configuration.
The fix is to disable exception logging temporarily while the logger
is constructed.

git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/0.10@1083897 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 62 insertions, 5 deletions

diff --git a/qpid/cpp/src/qpid/DisableExceptionLogging.h b/qpid/cpp/src/qpid/DisableExceptionLogging.h
new file mode 100644
index 0000000000..04a9240513
--- /dev/null
+++ b/qpid/cpp/src/qpid/DisableExceptionLogging.h
@@ -0,0 +1,39 @@
+#ifndef QPID_DISABLEEXCEPTIONLOGGING_H
+#define QPID_DISABLEEXCEPTIONLOGGING_H
+
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+#include "qpid/CommonImportExport.h"
+
+namespace qpid {
+
+/**
+ * Temporarily disable logging in qpid::Exception constructor.
+ * Used by log::Logger to avoid logging exceptions during Logger construction.
+ */
+struct DisableExceptionLogging
+{
+    QPID_COMMON_EXTERN DisableExceptionLogging();
+    QPID_COMMON_EXTERN ~DisableExceptionLogging();
+};
+} // namespace qpid
+
+#endif  /*!QPID_DISABLEEXCEPTIONLOGGING_H*/
diff --git a/qpid/cpp/src/qpid/Exception.cpp b/qpid/cpp/src/qpid/Exception.cpp
index 16a3a13d17..a6696f06e1 100644
--- a/qpid/cpp/src/qpid/Exception.cpp
+++ b/qpid/cpp/src/qpid/Exception.cpp
@@ -7,9 +7,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -21,13 +21,25 @@
 
 #include "qpid/log/Statement.h"
 #include "qpid/Exception.h"
+#include "qpid/DisableExceptionLogging.h"
 #include <typeinfo>
 #include <assert.h>
 #include <string.h>
 
 namespace qpid {
 
+// Note on static initialization order: if an exception is constructed
+// in a static constructor before disableExceptionLogging has been
+// initialized, the worst that can happen is we lose an exception log
+// message. Since we shouldn't be throwing a lot of exceptions during
+// static construction this seems safe.
+static bool disableExceptionLogging = false;
+
+DisableExceptionLogging::DisableExceptionLogging() { disableExceptionLogging = true; }
+DisableExceptionLogging::~DisableExceptionLogging() { disableExceptionLogging = false; }
+
 Exception::Exception(const std::string& msg) throw() : message(msg) {
+    if (disableExceptionLogging) return;
     QPID_LOG_IF(debug, !msg.empty(), "Exception constructed: " << message);
 }
 
diff --git a/qpid/cpp/src/qpid/log/Logger.cpp b/qpid/cpp/src/qpid/log/Logger.cpp
index 2217cdddbd..2339a62114 100644
--- a/qpid/cpp/src/qpid/log/Logger.cpp
+++ b/qpid/cpp/src/qpid/log/Logger.cpp
@@ -22,6 +22,7 @@
 #include "qpid/memory.h"
 #include "qpid/sys/Thread.h"
 #include "qpid/sys/Time.h"
+#include "qpid/DisableExceptionLogging.h"
 #include <boost/pool/detail/singleton.hpp>
 #include <boost/bind.hpp>
 #include <boost/function.hpp>
@@ -48,11 +49,16 @@ Logger& Logger::instance() {
 }
 
 Logger::Logger() : flags(0) {
+    // Disable automatic logging in Exception constructors to avoid
+    // re-entrant use of logger singleton if there is an error in
+    // option parsing.
+    DisableExceptionLogging del;
+
     // Initialize myself from env variables so all programs
     // (e.g. tests) can use logging even if they don't parse
     // command line args.
     Options opts("");
-    opts.parse(0, 0);           
+    opts.parse(0, 0);
     configure(opts);
 }
 
@@ -73,7 +79,7 @@ void Logger::log(const Statement& s, const std::string& msg) {
     std::ostringstream os;
     if (!prefix.empty())
         os << prefix << ": ";
-    if (flags&TIME) 
+    if (flags&TIME)
 		qpid::sys::outputFormattedNow(os);
     if (flags&LEVEL)
         os << LevelTraits::name(s.level) << " ";
@@ -140,7 +146,7 @@ void Logger::configure(const Options& opts) {
     Options o(opts);
     if (o.trace)
         o.selectors.push_back("trace+");
-    format(o); 
+    format(o);
     select(Selector(o));
     setPrefix(opts.prefix);
     options.sinkOptions->setup(this);