Fix race condition in OCSPRequestSession.

net/ocsp/nss_ocsp.cc

 // Copyright (c) 2009 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 "net/ocsp/nss_ocsp.h"
 
 // Work around https://bugzilla.mozilla.org/show_bug.cgi?id=455424
 // until NSS 3.12.2 comes out and we update to it.
 #define Lock FOO_NSS_Lock
 #include <certt.h>
@@ skipping 75 matching lines @@
     : public base::RefCountedThreadSafe<OCSPRequestSession>,
       public URLRequest::Delegate,
       public MessageLoop::DestructionObserver {
  public:
   OCSPRequestSession(const GURL& url,
                      const char* http_request_method,
                      base::TimeDelta timeout)
       : url_(url),
         http_request_method_(http_request_method),
         timeout_(timeout),
-        io_loop_(Singleton<OCSPInitSingleton>::get()->io_thread()),
         request_(NULL),
         buffer_(new net::IOBuffer(kRecvBufferSize)),
         response_code_(-1),
         cv_(&lock_),
+        io_loop_(Singleton<OCSPInitSingleton>::get()->io_thread()),
         finished_(false) {}
 
   void SetPostData(const char* http_data, PRUint32 http_data_len,
                    const char* http_content_type) {
     upload_content_.assign(http_data, http_data_len);
     upload_content_type_.assign(http_content_type);
   }
 
   void AddHeader(const char* http_header_name, const char* http_header_value) {
     if (!extra_request_headers_.empty())
       extra_request_headers_ += "\r\n";
     StringAppendF(&extra_request_headers_,
                   "%s: %s", http_header_name, http_header_value);
   }
 
   void Start() {
+    // IO thread may set |io_loop_| to NULL, so protect by |lock_|.
+    AutoLock autolock(lock_);
     if (io_loop_) {
       io_loop_->PostTask(
           FROM_HERE,
           NewRunnableMethod(this, &OCSPRequestSession::StartURLRequest));
     }
   }
 
   bool Started() const {
     return request_ != NULL;
   }
 
   void Cancel() {
-    if (io_loop_ && request_) {
+    // IO thread may set |io_loop_| to NULL, so protect by |lock_|.
+    AutoLock autolock(lock_);
+    if (io_loop_) {
       io_loop_->PostTask(
           FROM_HERE,
           NewRunnableMethod(this, &OCSPRequestSession::CancelURLRequest));
     }
   }
 
   bool Finished() const {
     AutoLock autolock(lock_);
     return finished_;
   }
@@ skipping 67 matching lines @@
     DCHECK_EQ(request, request_);
     DCHECK_EQ(MessageLoopForIO::current(), io_loop_);
 
     do {
       if (!request_->status().is_success() || bytes_read <= 0)
         break;
       data_.append(buffer_->data(), bytes_read);
     } while (request_->Read(buffer_, kRecvBufferSize, &bytes_read));
 
     if (!request_->status().is_io_pending()) {
+      MessageLoop* io_loop = io_loop_;
       {
         AutoLock autolock(lock_);
         finished_ = true;
+        io_loop_ = NULL;
       }
       cv_.Signal();
       delete request_;
       request_ = NULL;
-      io_loop_->RemoveDestructionObserver(this);
-      io_loop_ = NULL;
+      io_loop->RemoveDestructionObserver(this);
     }
   }
 
   virtual void WillDestroyCurrentMessageLoop() {
     DCHECK_EQ(MessageLoopForIO::current(), io_loop_);
+    {
+      AutoLock autolock(lock_);
+      io_loop_ = NULL;
+    }
     if (request_) {
       request_->Cancel();
       delete request_;
       request_ = NULL;
     }
-    io_loop_ = NULL;
   }
 
  private:
   friend class base::RefCountedThreadSafe<OCSPRequestSession>;
 
   virtual ~OCSPRequestSession() {
     DCHECK(!request_);
     if (io_loop_)
       io_loop_->RemoveDestructionObserver(this);
-    io_loop_ = NULL;
   }
 
   void StartURLRequest() {
     DCHECK_EQ(MessageLoopForIO::current(), io_loop_);
     DCHECK(!request_);
 
     io_loop_->AddDestructionObserver(this);
 
     request_ = new URLRequest(url_, this);
     request_->set_context(
@@ skipping 14 matching lines @@
       request_->AppendBytesToUpload(upload_content_.data(),
                                     static_cast<int>(upload_content_.size()));
     }
     if (!extra_request_headers_.empty())
       request_->SetExtraRequestHeaders(extra_request_headers_);
 
     request_->Start();
   }
 
   void CancelURLRequest() {
-    DCHECK_EQ(MessageLoopForIO::current(), io_loop_);
+    if (io_loop_)
+      DCHECK_EQ(MessageLoopForIO::current(), io_loop_);
     if (request_) {
       request_->Cancel();
       delete request_;
       request_ = NULL;
     }
   }
 
   GURL url_;                      // The URL we eventually wound up at
   std::string http_request_method_;
   base::TimeDelta timeout_;       // The timeout for OCSP
-  MessageLoop* io_loop_;          // Message loop of the IO thread
   URLRequest* request_;           // The actual request this wraps
   scoped_refptr<net::IOBuffer> buffer_;  // Read buffer
   std::string extra_request_headers_;  // Extra headers for the request, if any
   std::string upload_content_;    // HTTP POST payload
   std::string upload_content_type_;  // MIME type of POST payload
 
   int response_code_;             // HTTP status code for the request
   std::string response_content_type_;
   scoped_refptr<net::HttpResponseHeaders> response_headers_;
   std::string data_;              // Results of the requst
 
-  // |lock_| protects |finished_| only.
+  // |lock_| protects |finished_| and |io_loop_|.
   mutable Lock lock_;
   ConditionVariable cv_;
 
+  MessageLoop* io_loop_;          // Message loop of the IO thread
   bool finished_;
 
   DISALLOW_COPY_AND_ASSIGN(OCSPRequestSession);
 };
 
 // Concrete class for SEC_HTTP_SERVER_SESSION.
 class OCSPServerSession {
  public:
   OCSPServerSession(const char* host, PRUint16 port)
       : host_(host), port_(port) {}
@@ skipping 235 matching lines @@
 // This function would be called before NSS initialization.
 void SetURLRequestContextForOCSP(URLRequestContext* request_context) {
   OCSPInitSingleton::set_url_request_context(request_context);
 }
 
 URLRequestContext* GetURLRequestContextForOCSP() {
   return OCSPInitSingleton::url_request_context();
 }
 
 }  // namespace net