From 9b5c807272e1f470ece3d5a313041f4074bfdd53 Mon Sep 17 00:00:00 2001
From: "pbos@webrtc.org" <pbos@webrtc.org@4adac7df-926f-26a2-2b94-8c16560cd09d>
Date: Wed, 2 Oct 2013 16:22:18 +0000
Subject: [PATCH] Remove ReturnTrace from DeregisterCallback().

Should fix deadlock on build bots. Before, TraceImpl called
TraceDispatcher::Print, while TraceDispatcher::Deregister called
TraceImpl through VideoEngine::SetTraceCallback. This violates locking
order as both take their own locks.

BUG=2421
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/2340005

git-svn-id: http://webrtc.googlecode.com/svn/trunk@4905 4adac7df-926f-26a2-2b94-8c16560cd09d
---
 webrtc/video_engine/internal/call.cc | 30 ++++++++++++++--------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/webrtc/video_engine/internal/call.cc b/webrtc/video_engine/internal/call.cc
index c3780fdc48..8b7f661dd2 100644
--- a/webrtc/video_engine/internal/call.cc
+++ b/webrtc/video_engine/internal/call.cc
@@ -31,8 +31,16 @@ class TraceDispatcher : public TraceCallback {
  public:
   TraceDispatcher()
       : crit_(CriticalSectionWrapper::CreateCriticalSection()),
+        initialized_(false),
         filter_(kTraceNone) {}
 
+  ~TraceDispatcher() {
+    if (initialized_) {
+      Trace::ReturnTrace();
+      VideoEngine::SetTraceCallback(NULL);
+    }
+  }
+
   virtual void Print(TraceLevel level,
                      const char* message,
                      int length) OVERRIDE {
@@ -52,23 +60,18 @@ class TraceDispatcher : public TraceCallback {
     CriticalSectionScoped lock(crit_.get());
     callbacks_[call] = config;
 
-    if ((filter_ | config->trace_filter) != filter_) {
-      if (filter_ == kTraceNone) {
-        Trace::CreateTrace();
-        VideoEngine::SetTraceCallback(this);
-      }
-      filter_ |= config->trace_filter;
-      VideoEngine::SetTraceFilter(filter_);
+    filter_ |= config->trace_filter;
+    if (filter_ != kTraceNone && !initialized_) {
+      initialized_ = true;
+      Trace::CreateTrace();
+      VideoEngine::SetTraceCallback(this);
     }
+    VideoEngine::SetTraceFilter(filter_);
   }
 
   void DeregisterCallback(Call* call) {
     CriticalSectionScoped lock(crit_.get());
     callbacks_.erase(call);
-    // Return early if there was no filter, this is required to prevent
-    // returning the Trace handle more than once.
-    if (filter_ == kTraceNone)
-      return;
 
     filter_ = kTraceNone;
     for (std::map<Call*, Call::Config*>::iterator it = callbacks_.begin();
@@ -78,14 +81,11 @@ class TraceDispatcher : public TraceCallback {
     }
 
     VideoEngine::SetTraceFilter(filter_);
-    if (filter_ == kTraceNone) {
-      VideoEngine::SetTraceCallback(NULL);
-      Trace::ReturnTrace();
-    }
   }
 
  private:
   scoped_ptr<CriticalSectionWrapper> crit_;
+  bool initialized_;
   unsigned int filter_;
   std::map<Call*, Call::Config*> callbacks_;
 };