[Draft] Fix canceling pin prompt causes host overload

remoting/protocol/jingle_session.cc

Index: remoting/protocol/jingle_session.cc
diff --git a/remoting/protocol/jingle_session.cc b/remoting/protocol/jingle_session.cc
index 2aecd4c25d6856d86a4f8f0ab1645219edad48a1..379ad9760fff6b6110c298826968f149ba6b147d 100644
--- a/remoting/protocol/jingle_session.cc
+++ b/remoting/protocol/jingle_session.cc
@@ -6,8 +6,10 @@
 
 #include "base/bind.h"
 #include "base/rand_util.h"
+#include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "remoting/base/constants.h"
 #include "remoting/jingle_glue/iq_sender.h"
@@ -61,6 +63,7 @@ ErrorCode AuthRejectionReasonToErrorCode(
 JingleSession::JingleSession(JingleSessionManager* session_manager)
     : session_manager_(session_manager),
       event_handler_(NULL),
+      weak_factory_(this),
       state_(INITIALIZING),
       error_(OK),
       config_is_set_(false) {
@@ -180,10 +183,11 @@ void JingleSession::ContinueAcceptIncomingConnection() {
   if (authenticator_->state() == Authenticator::ACCEPTED) {
     SetState(AUTHENTICATED);
   } else {
+    if (authenticator_->started()) {
+      SetState(AUTHENTICATING);
+    }
     DCHECK_EQ(authenticator_->state(), Authenticator::WAITING_MESSAGE);
   }
-
-  return;
 }
 
 const std::string& JingleSession::jid() {
@@ -485,7 +489,7 @@ void JingleSession::OnSessionInfo(const JingleMessage& message,
     return;
   }
 
-  if (state_ != CONNECTED ||
+  if ((state_ != CONNECTED && state_ != AUTHENTICATING) ||
       authenticator_->state() != Authenticator::WAITING_MESSAGE) {
     LOG(WARNING) << "Received unexpected authenticator message "
                  << message.info->Str();
@@ -517,8 +521,7 @@ void JingleSession::ProcessTransportInfo(const JingleMessage& message) {
 
 void JingleSession::OnTerminate(const JingleMessage& message,
                                 const ReplyCallback& reply_callback) {
-  if (state_ != CONNECTING && state_ != ACCEPTING && state_ != CONNECTED &&
-      state_ != AUTHENTICATED) {
+  if (!is_session_active()) {
     LOG(WARNING) << "Received unexpected session-terminate message.";
     reply_callback.Run(JingleMessageReply::UNEXPECTED_REQUEST);
     return;
@@ -577,7 +580,7 @@ void JingleSession::ProcessAuthenticationStep() {
   DCHECK(CalledOnValidThread());
   DCHECK_NE(authenticator_->state(), Authenticator::PROCESSING_MESSAGE);
 
-  if (state_ != CONNECTED) {
+  if (state_ != CONNECTED && state_ != AUTHENTICATING) {
     DCHECK(state_ == FAILED || state_ == CLOSED);
     // The remote host closed the connection while the authentication was being
     // processed asynchronously, nothing to do.
@@ -592,6 +595,21 @@ void JingleSession::ProcessAuthenticationStep() {
   }
   DCHECK_NE(authenticator_->state(), Authenticator::MESSAGE_READY);
 
+  // The current JingleSession object can be destroyed by event_handler of
+  // SetState(AUTHENTICATING) and cause subsequent dereferencing of the this
+  // pointer to crash.  To protect against it, we run ContinueAuthenticationStep
+  // asychronously using a weak pointer.

[Sergey Ulanov, 2014/03/27 19:06:55]

Please add a unittests that tries to delete JingleSession on AUTHENTICATING
state change

[kelvinp, 2014/04/01 21:23:49]

Done.

+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+    FROM_HERE,
+    base::Bind(&JingleSession::ContinueAuthenticationStep,
+               weak_factory_.GetWeakPtr()));
+
+  if (authenticator_->started()) {
+    SetState(AUTHENTICATING);
+  }
+}
+
+void JingleSession::ContinueAuthenticationStep() {
   if (authenticator_->state() == Authenticator::ACCEPTED) {
     SetState(AUTHENTICATED);
   } else if (authenticator_->state() == Authenticator::REJECTED) {
@@ -603,8 +621,7 @@ void JingleSession::ProcessAuthenticationStep() {
 void JingleSession::CloseInternal(ErrorCode error) {
   DCHECK(CalledOnValidThread());
 
-  if (state_ == CONNECTING || state_ == ACCEPTING || state_ == CONNECTED ||
-      state_ == AUTHENTICATED) {
+  if (is_session_active()) {
     // Send session-terminate message with the appropriate error code.
     JingleMessage::Reason reason;
     switch (error) {
@@ -655,5 +672,10 @@ void JingleSession::SetState(State new_state) {
   }
 }
 
+bool JingleSession::is_session_active() {
+  return state_ == CONNECTING || state_ == ACCEPTING || state_ == CONNECTED ||
+        state_ == AUTHENTICATING || state_ == AUTHENTICATED;
+}
+
 }  // namespace protocol
 }  // namespace remoting