Allow multiple trace client callbacks (#844)

* Allow multiple trace client callbacks

Include/httpClient/trace.h

@@ -195,7 +195,8 @@ typedef void (CALLBACK HCTraceCallback)(
     );
 
 /// <summary>
-/// Set client callback for tracing.
+/// Set a client callback for tracing. Note that once a trace callback is set, it may be
+/// called until the library is fully cleaned up.
 /// </summary>
 /// <param name="callback">Trace callback.</param>
 /// <returns></returns>

Source/Logger/trace.cpp

@@ -194,10 +194,13 @@ void TraceMessageToClient(
     char const* message
 ) noexcept
 {
-    auto callback = GetTraceState().GetClientCallback();
-    if (callback)
+    TraceState& traceState{ GetTraceState() };
+    for (size_t i = 0; i < MAX_TRACE_CLIENTS; ++i)
     {
-        callback(areaName, level, threadId, timestamp, message);
+        if (traceState.clientCallbacks[i])
+        {
+            traceState.clientCallbacks[i](areaName, level, threadId, timestamp, message);
+        }
     }
 }
 
@@ -271,6 +274,8 @@ STDAPI_(void) HCTraceImplMessage_v(
     va_list varArgs
 ) noexcept
 {
+    TraceState& traceState{ GetTraceState() };
+
     if (!area)
     {
         return;
@@ -281,7 +286,7 @@ STDAPI_(void) HCTraceImplMessage_v(
         return;
     }
 
-    if (!GetTraceState().IsSetup())
+    if (!traceState.IsSetup())
     {
         return;
     }
@@ -292,7 +297,17 @@ STDAPI_(void) HCTraceImplMessage_v(
     }
 
     // Only do work if there's reason to
-    if (GetTraceState().GetClientCallback() == nullptr && !GetTraceState().GetTraceToDebugger() && !GetTraceState().GetEtwEnabled())
+    bool haveClientCallback = false;
+    for (size_t i = 0; i < MAX_TRACE_CLIENTS; ++i)
+    {
+        if (traceState.clientCallbacks[i])
+        {
+            haveClientCallback = true;
+            break;
+        }
+    }
+
+    if (!haveClientCallback && !traceState.GetTraceToDebugger() && !traceState.GetEtwEnabled())
     {
         return;
     }
@@ -368,12 +383,16 @@ void TraceState::SetEtwEnabled(_In_ bool etwEnabled) noexcept
 
 void TraceState::SetClientCallback(HCTraceCallback* callback) noexcept
 {
-    m_clientCallback = callback;
-}
-
-HCTraceCallback* TraceState::GetClientCallback() const noexcept
-{
-    return m_clientCallback;
+    // Try to add a client callback. If MAX_TRACE_CLIENTS have already set callbacks, the callback won't be set
+    // and the client will not get trace callbacks.
+    for (size_t i = 0; i < MAX_TRACE_CLIENTS; ++i)
+    {
+        if (clientCallbacks[i] == nullptr)
+        {
+            clientCallbacks[i] = callback;
+            break;
+        }
+    }
 }
 
 uint64_t TraceState::GetTimestamp() const noexcept

Source/Logger/trace_internal.h

@@ -2,6 +2,8 @@
 
 #include <httpClient/trace.h>
 
+#define MAX_TRACE_CLIENTS 10
+
 class TraceState
 {
 public:
@@ -12,20 +14,20 @@ public:
     bool GetTraceToDebugger() noexcept;
     void SetTraceToDebugger(_In_ bool traceToDebugger) noexcept;
     void SetClientCallback(HCTraceCallback* callback) noexcept;
-    HCTraceCallback* GetClientCallback() const noexcept;
     uint64_t GetTimestamp() const noexcept;
     bool GetEtwEnabled() const noexcept;
 #if HC_PLATFORM_IS_MICROSOFT
     void SetEtwEnabled(_In_ bool enabled) noexcept;
 #endif
 
+    HCTraceCallback* clientCallbacks[MAX_TRACE_CLIENTS]{};
+
 private:
     std::atomic<uint32_t> m_tracingClients{ 0 };
     std::atomic<std::chrono::high_resolution_clock::time_point> m_initTime
     {
         std::chrono::high_resolution_clock::time_point{}
     };
-    std::atomic<HCTraceCallback*> m_clientCallback{ nullptr };
     bool m_traceToDebugger = false;
     bool m_etwEnabled = false;
 };

Source/Task/ThreadPool_stl.cpp

@@ -96,7 +96,7 @@ namespace OS
                                 // calls.
 
                                 {
-                                    std::unique_lock<std::mutex> lock(m_activeLock);
+                                    std::unique_lock<std::mutex> lock2(m_activeLock);
                                     m_activeCalls++;
                                 }