Issue #1234, #2058 - Rewrite log facility, improving thread implementation

src/plugin/PluginDebug.h

Index: src/plugin/PluginDebug.h
===================================================================
--- a/src/plugin/PluginDebug.h
+++ b/src/plugin/PluginDebug.h
@@ -18,17 +18,65 @@
 #ifndef _PLUGIN_DEBUG_H_
 #define _PLUGIN_DEBUG_H_
 
+#include <memory>
+#include <mutex>
+
+namespace Trace
+{
+  /**
+   */
+  struct Location
+  {
+    std::string prefixText;
+    std::string postfixText;
+
+    Location() // = default
+      : prefixText(), postfixText()
+    {};
+
+    Location(int id, int subId);
+
+    Location(std::string postfixText)
+      : prefixText(), postfixText(postfixText)
+    {}
+
+    Location(std::string prefixText, std::string postfixText)
+      : prefixText(prefixText), postfixText(postfixText)
+    {}
+  };
+
+  void TextFixed(const std::string& description, Trace::Location location);
+  void ErrorCode(DWORD errorCode, const std::string& description, Trace::Location location);
+  void SystemException(const std::system_error& ex, const std::string& description, Trace::Location location);
+}
+
+// Cope with insufficient support on old toolsets
+#if !defined(__func__) && defined(_MSC_VER)
+#define __func__ __FUNCTION__
+#endif
+
+// __LINE__ expands to an integer literal, not a string literal
+// The stringify operator "#" applies only to arguments, not macro definitions
+#define STRING_EXPAND(x) STRINGIFY(x)
+#define STRINGIFY(x) #x
+#define __LINE_STRING__ STRING_EXPAND(__LINE__)
+#define HERE Trace::Location(__FILE__ ":" __LINE_STRING__)
+#define HERE_F Trace::Location(__func__, __FILE__ ":" __LINE_STRING__)
+
+#ifdef _DEBUG
+#define TRACE(description, location) Trace::TextFixed(description, location)
+#else
+#define TRACE(a,b)
+#endif
+
 class CPluginDebug
 {
-
 public:
-  static void DebugSystemException(const std::system_error& ex, int errorId, int errorSubid, const std::string& description); 
-
 #if (defined ENABLE_DEBUG_INFO)
   static void Debug(const std::string& text);
   static void Debug(const std::wstring& text);
-  static void DebugException(const std::exception& ex);
-  static void DebugErrorCode(DWORD errorCode, const std::string& error, DWORD processId=0, DWORD threadId=0);
+  static void DebugException(const std::exception& ex, Trace::Location location);
+  static void DebugErrorCode(DWORD errorCode, const std::string& error, Trace::Location location);
 #endif
 
 #if (defined ENABLE_DEBUG_RESULT)
@@ -43,5 +91,59 @@
 #endif
 };
 
+/*
+ * Forward declaration.
+ */
+class LogQueue;
+
+/**
+ * This class maintains a singleton LogQueue in existence.
+ *
+ * This class exists because it's not possible to use a static declaration for
+ *  'std::thread' or any class that contains one, such as 'ActiveQueue'.
+ * To appreciate of the origin of restriction, see 
+ *   http://stackoverflow.com/questions/28746016/thread-join-does-not-return-when-called-in-global-var-destructor
+ * In addition, we have no main() function to use for initialization,
+ *   since we're packaged as a COM class provider.
+ *
+ * As a substitute, we maintain a reference to the queue in each instance of
+ *   'CPluginClass', the sole visible COM class, the one that implements the
+ *   BHO interface.
+ * This class has the responsibility for managing the life cycle of the queue,
+ *   ensuring it exists whenever there's a reference to it.
+ * The lifespan of instances of 'CPluginClass' encompasses the lifespans of any
+ *   other objects that the BHO uses, so it's sufficient to maintain a LogQueue
+ *   in existence with a reference in that class.
+ *
+ * The queue itself is accessible only to the logging functions;
+ *   it need not be visible externally.
+ * Hence all this class requires is an opaque, forward declaration.
+ * Mere instantiation of this class is sufficient to ensure we have a log queue.
+ */
+class LogQueueReference
+{
+  friend LogQueue;
+
+  typedef std::shared_ptr<LogQueue> pointer;
+
+  /**
+   * The class-wide reference to the debug queue.
+   */
+  static pointer queueMasterReference;

[sergei, 2015/10/01 15:50:07]

Why do we need more static stuff? We have already singletons for example
CAdblockPlusClient and CPluginSettings, why not to combine them all in one
singleton?

[Eric, 2015/10/08 21:05:41]

In the present case, we want a single log facility for the whole DLL. Read the
comment above on why this isn't a simple static declaration, which would indeed
be simpler.
How would moving things out of their own scopes and creating a new _de facto_
global scope provide any benefit whatsoever? There's a reason for the consensus
that global variables are bad. When you've got separable responsibilities, you
put them in separate objects.

In fact, the lifetimes of these other objects do _not_ coincide with that of the
log queue. The reason is in the comment above, because you can't statically
initialize a thread in a DLL. Even if combining everything were a good idea
(which it's not), you'd need to seriously rework the initialization of these
other classes.

+
+  /**
+   * The instance reference to the debug queue.
+   */
+  pointer queueReference;
+
+  /**
+   * Mutex to allow joint serialization of constructor/destructor calls.
+   */
+  static std::mutex mutex;
+
+public:
+  LogQueueReference();
+  ~LogQueueReference();
+};
 
 #endif // _PLUGIN_DEBUG_H_