Properly handle screen recorder shutdown in ChromotingHost.

remoting/host/chromoting_host.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/host/chromoting_host.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "build/build_config.h"
@@ skipping 37 matching lines @@
                                MutableHostConfig* config,
                                DesktopEnvironment* environment,
                                AccessVerifier* access_verifier,
                                bool allow_nat_traversal)
     : context_(context),
       desktop_environment_(environment),
       config_(config),
       access_verifier_(access_verifier),
       allow_nat_traversal_(allow_nat_traversal),
       state_(kInitial),
+      stopping_recorders_(0),
       protocol_config_(protocol::CandidateSessionConfig::CreateDefault()),
       is_curtained_(false),
       is_it2me_(false) {
   DCHECK(desktop_environment_);
   desktop_environment_->set_host(this);
 }
 
 ChromotingHost::~ChromotingHost() {
 }
 
@@ skipping 301 matching lines @@
   }
   if (client == clients_.end())
     return;
 
   // Remove the connection from the session manager and stop the session.
   // TODO(hclam): Stop only if the last connection disconnected.
   if (recorder_.get()) {
     recorder_->RemoveConnection(connection);
     // The recorder only exists to serve the unique authenticated client.
     // If that client has disconnected, then we can kill the recorder.
-    if (client->get()->authenticated()) {
-      recorder_->Stop(NULL);
-      recorder_ = NULL;
-    }
+    if (client->get()->authenticated())
+      StopScreenRecorder();
   }
 
-  // Close the connection to connection just to be safe.
+  // Close the connection to client just to be safe.
   connection->Disconnect();
 
   // Also remove reference to ConnectionToClient from this object.
   int old_authenticated_clients = AuthenticatedClientsCount();
   clients_.erase(client);
 
   // Notify the observers of the change, if any.
   int authenticated_clients = AuthenticatedClientsCount();
   if (old_authenticated_clients != authenticated_clients) {
     for (StatusObserverList::iterator it = status_observers_.begin();
@@ skipping 139 matching lines @@
   // Find the client session corresponding to the given connection.
   ClientList::iterator client;
   for (client = clients_.begin(); client != clients_.end(); ++client) {
     if (client->get()->connection() == connection)
       break;
   }
   CHECK(client != clients_.end());
   client->get()->OnAuthorizationComplete(true);
 }
 
+void ChromotingHost::StopScreenRecorder() {
+  DCHECK(MessageLoop::current() == context_->main_message_loop());
+
+  ++stopping_recorders_;
+  recorder_->Stop(base::Bind(&ChromotingHost::OnScreenRecorderStopped, this));

[dmac, 2011/08/11 23:56:34]

should we check recorder_.get() != NULL? Could we get in here again?

[Sergey Ulanov, 2011/08/12 00:29:48]

Done.

+  recorder_ = NULL;
+}
+
+void ChromotingHost::OnScreenRecorderStopped() {
+  if (MessageLoop::current() != context_->main_message_loop()) {
+    context_->main_message_loop()->PostTask(
+        FROM_HERE, base::Bind(&ChromotingHost::OnScreenRecorderStopped, this));
+    return;
+  }
+
+  --stopping_recorders_;
+  DCHECK_GE(stopping_recorders_, 0);
+
+  bool stopping;
+  {
+    base::AutoLock auto_lock(lock_);
+    stopping = state_ == kStopping;
+  }
+
+  if (!stopping_recorders_ && stopping)
+    ShutdownFinish();
+}
+
 void ChromotingHost::ShutdownNetwork() {
   if (MessageLoop::current() != context_->network_message_loop()) {
     context_->network_message_loop()->PostTask(
         FROM_HERE, base::Bind(&ChromotingHost::ShutdownNetwork, this));
     return;
   }
 
   // Stop chromotocol session manager.
   if (session_manager_.get()) {
     session_manager_->Close();
@@ skipping 15 matching lines @@
 }
 
 void ChromotingHost::ShutdownRecorder() {
   if (MessageLoop::current() != context_->main_message_loop()) {
     context_->main_message_loop()->PostTask(
         FROM_HERE, base::Bind(&ChromotingHost::ShutdownRecorder, this));
     return;
   }
 
   if (recorder_.get()) {
-    recorder_->Stop(NewRunnableMethod(this, &ChromotingHost::ShutdownFinish));
-  } else {
+    StopScreenRecorder();
+  } else if (!stopping_recorders_) {
     ShutdownFinish();
   }
 }
 
 void ChromotingHost::ShutdownFinish() {
   if (MessageLoop::current() != context_->main_message_loop()) {
     context_->main_message_loop()->PostTask(
         FROM_HERE, base::Bind(&ChromotingHost::ShutdownFinish, this));
     return;
   }
@@ skipping 11 matching lines @@
 
   for (std::vector<Task*>::iterator it = shutdown_tasks_.begin();
        it != shutdown_tasks_.end(); ++it) {
     (*it)->Run();
     delete *it;
   }
   shutdown_tasks_.clear();
 }
 
 }  // namespace remoting