AddRef() / Release() while URLRequest is alive in OCSPRequestSession

net/ocsp/nss_ocsp.cc

Index: net/ocsp/nss_ocsp.cc
diff --git a/net/ocsp/nss_ocsp.cc b/net/ocsp/nss_ocsp.cc
index e48edc102839457738444180051e1068d74ff017..3f4daee99ec06d7ad8efabdaa05be6d63c0c7f20 100644
--- a/net/ocsp/nss_ocsp.cc
+++ b/net/ocsp/nss_ocsp.cc
@@ -128,11 +128,7 @@ class OCSPRequestSession
   void Cancel() {
     // 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));
-    }
+    CancelLocked();
   }
 
   bool Finished() const {
@@ -152,7 +148,7 @@ class OCSPRequestSession
       if (timeout < base::TimeDelta()) {
         LOG(INFO) << "OCSP Timed out";
         if (!finished_)
-          Cancel();
+          CancelLocked();
         break;
       }
     }
@@ -225,6 +221,7 @@ class OCSPRequestSession
         io_loop_ = NULL;
       }
       cv_.Signal();
+      Release();  // Balanced with StartURLRequest().
     }
   }
 
@@ -234,11 +231,7 @@ class OCSPRequestSession
       AutoLock autolock(lock_);
       io_loop_ = NULL;
     }
-    if (request_) {
-      request_->Cancel();
-      delete request_;
-      request_ = NULL;
-    }
+    CancelURLRequest();
   }
 
  private:
@@ -250,6 +243,14 @@ class OCSPRequestSession
       io_loop_->RemoveDestructionObserver(this);
   }
 
+  void CancelLocked() {

[wtc, 2009/12/08 01:44:01]

Please add a comment to document that we must call this method
while holding lock_.

You can add this comment to your other CL at
http://codereview.chromium.org/460067

Thanks!

[ukai, 2009/12/08 06:02:05]

Sure. Added lock_.AssertAcquired() too.

+    if (io_loop_) {
+      io_loop_->PostTask(
+          FROM_HERE,
+          NewRunnableMethod(this, &OCSPRequestSession::CancelURLRequest));
+    }
+  }
+
   void StartURLRequest() {
     DCHECK_EQ(MessageLoopForIO::current(), io_loop_);
     DCHECK(!request_);
@@ -279,6 +280,7 @@ class OCSPRequestSession
       request_->SetExtraRequestHeaders(extra_request_headers_);
 
     request_->Start();
+    AddRef();  // Release after |request_| deleted.
   }
 
   void CancelURLRequest() {
@@ -288,6 +290,13 @@ class OCSPRequestSession
       request_->Cancel();
       delete request_;
       request_ = NULL;
+      {
+        AutoLock autolock(lock_);
+        finished_ = true;
+        io_loop_ = NULL;
+      }
+      cv_.Signal();
+      Release();  // Balanced with StartURLRequest().
     }
   }