Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(135)

Side by Side Diff: net/base/cert_verifier.cc

Issue 4299001: Update CertVerifier to watch for the origin loop's destruction, so that (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Proper scoping of lock, static cast. Created 10 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « net/base/cert_verifier.h ('k') | tools/valgrind/gtest_exclude/net_unittests.gtest_mac.txt » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2008 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2008 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "net/base/cert_verifier.h" 5 #include "net/base/cert_verifier.h"
6 6
7 #if defined(USE_NSS) 7 #if defined(USE_NSS)
8 #include <private/pprthred.h> // PR_DetatchThread 8 #include <private/pprthred.h> // PR_DetatchThread
9 #endif 9 #endif
10 10
11 #include "base/message_loop.h" 11 #include "base/message_loop.h"
12 #include "base/worker_pool.h" 12 #include "base/worker_pool.h"
13 #include "net/base/cert_verify_result.h" 13 #include "net/base/cert_verify_result.h"
14 #include "net/base/net_errors.h" 14 #include "net/base/net_errors.h"
15 #include "net/base/x509_certificate.h" 15 #include "net/base/x509_certificate.h"
16 16
17 namespace net { 17 namespace net {
18 18
19 class CertVerifier::Request : 19 class CertVerifier::Request :
20 public base::RefCountedThreadSafe<CertVerifier::Request> { 20 public base::RefCountedThreadSafe<CertVerifier::Request,
21 CertVerifier::RequestTraits>,
22 public MessageLoop::DestructionObserver {
21 public: 23 public:
22 Request(CertVerifier* verifier, 24 Request(CertVerifier* verifier,
23 X509Certificate* cert, 25 X509Certificate* cert,
24 const std::string& hostname, 26 const std::string& hostname,
25 int flags, 27 int flags,
26 CertVerifyResult* verify_result, 28 CertVerifyResult* verify_result,
27 CompletionCallback* callback) 29 CompletionCallback* callback)
28 : cert_(cert), 30 : cert_(cert),
29 hostname_(hostname), 31 hostname_(hostname),
30 flags_(flags), 32 flags_(flags),
31 verifier_(verifier), 33 verifier_(verifier),
32 verify_result_(verify_result), 34 verify_result_(verify_result),
33 callback_(callback), 35 callback_(callback),
34 origin_loop_(MessageLoop::current()), 36 origin_loop_(MessageLoop::current()),
35 error_(OK) { 37 error_(OK) {
38 if (origin_loop_)
wtc 2010/11/15 23:57:15 Let's remove this null pointer check for origin_lo
Zachary Kuznia 2010/11/16 01:14:39 Done.
39 origin_loop_->AddDestructionObserver(this);
36 } 40 }
37 41
38 void DoVerify() { 42 void DoVerify() {
39 // Running on the worker thread 43 // Running on the worker thread
40 error_ = cert_->Verify(hostname_, flags_, &result_); 44 error_ = cert_->Verify(hostname_, flags_, &result_);
41 #if defined(USE_NSS) 45 #if defined(USE_NSS)
42 // Detach the thread from NSPR. 46 // Detach the thread from NSPR.
43 // Calling NSS functions attaches the thread to NSPR, which stores 47 // Calling NSS functions attaches the thread to NSPR, which stores
44 // the NSPR thread ID in thread-specific data. 48 // the NSPR thread ID in thread-specific data.
45 // The threads in our thread pool terminate after we have called 49 // The threads in our thread pool terminate after we have called
(...skipping 31 matching lines...) Expand 10 before | Expand all | Expand 10 after
77 // Drop the verifier's reference to us. Do this before running the 81 // Drop the verifier's reference to us. Do this before running the
78 // callback since the callback might result in the verifier being 82 // callback since the callback might result in the verifier being
79 // destroyed. 83 // destroyed.
80 verifier_->request_ = NULL; 84 verifier_->request_ = NULL;
81 85
82 callback_->Run(error_); 86 callback_->Run(error_);
83 } 87 }
84 88
85 void Cancel() { 89 void Cancel() {
86 verifier_ = NULL; 90 verifier_ = NULL;
91 AutoLock locked(origin_loop_lock_);
92 if (origin_loop_) {
93 origin_loop_->RemoveDestructionObserver(this);
94 origin_loop_ = NULL;
95 }
96 }
87 97
98 // MessageLoop::DestructionObserver override.
99 virtual void WillDestroyCurrentMessageLoop() {
100 LOG(ERROR) << "CertVerifier wasn't deleted before the thread was deleted.";
88 AutoLock locked(origin_loop_lock_); 101 AutoLock locked(origin_loop_lock_);
89 origin_loop_ = NULL; 102 origin_loop_ = NULL;
90 } 103 }
91 104
92 private: 105 private:
93 friend class base::RefCountedThreadSafe<CertVerifier::Request>; 106 friend class base::RefCountedThreadSafe<CertVerifier::Request>;
107 friend class DeleteTask<CertVerifier::Request>;
108 friend struct CertVerifier::RequestTraits;
94 109
95 ~Request() {} 110 ~Request() {
111 Cancel();
112 }
113
114 // If we have an origin_loop_, the observer needs to be removed on that
115 // thread. OnDestruct is called by RequestTraits to ensure this
116 // happens.
117 void OnDestruct() {
118 {
119 AutoLock locked(origin_loop_lock_);
120 if (origin_loop_ && origin_loop_ != MessageLoop::current()) {
121 origin_loop_->DeleteSoon(FROM_HERE, this);
122 return;
123 }
124 }
125 delete this;
126 }
96 127
97 // Set on the origin thread, read on the worker thread. 128 // Set on the origin thread, read on the worker thread.
98 scoped_refptr<X509Certificate> cert_; 129 scoped_refptr<X509Certificate> cert_;
99 std::string hostname_; 130 std::string hostname_;
100 // bitwise OR'd of X509Certificate::VerifyFlags. 131 // bitwise OR'd of X509Certificate::VerifyFlags.
101 int flags_; 132 int flags_;
102 133
103 // Only used on the origin thread (where Verify was called). 134 // Only used on the origin thread (where Verify was called).
104 CertVerifier* verifier_; 135 CertVerifier* verifier_;
105 CertVerifyResult* verify_result_; 136 CertVerifyResult* verify_result_;
106 CompletionCallback* callback_; 137 CompletionCallback* callback_;
107 138
108 // Used to post ourselves onto the origin thread. 139 // Used to post ourselves onto the origin thread.
109 Lock origin_loop_lock_; 140 Lock origin_loop_lock_;
110 MessageLoop* origin_loop_; 141 MessageLoop* origin_loop_;
111 142
112 // Assigned on the worker thread, read on the origin thread. 143 // Assigned on the worker thread, read on the origin thread.
113 int error_; 144 int error_;
114 CertVerifyResult result_; 145 CertVerifyResult result_;
115 }; 146 };
116 147
148 struct CertVerifier::RequestTraits {
149 static void Destruct(const CertVerifier::Request* request) {
150 const_cast<CertVerifier::Request*>(request)->OnDestruct();
151 }
152 };
153
117 //----------------------------------------------------------------------------- 154 //-----------------------------------------------------------------------------
118 155
119 CertVerifier::CertVerifier() { 156 CertVerifier::CertVerifier() {
120 } 157 }
121 158
122 CertVerifier::~CertVerifier() { 159 CertVerifier::~CertVerifier() {
123 if (request_) 160 if (request_)
124 request_->Cancel(); 161 request_->Cancel();
125 } 162 }
126 163
(...skipping 19 matching lines...) Expand all
146 NewRunnableMethod(request_.get(), &Request::DoVerify), true)) { 183 NewRunnableMethod(request_.get(), &Request::DoVerify), true)) {
147 NOTREACHED(); 184 NOTREACHED();
148 request_ = NULL; 185 request_ = NULL;
149 return ERR_FAILED; 186 return ERR_FAILED;
150 } 187 }
151 188
152 return ERR_IO_PENDING; 189 return ERR_IO_PENDING;
153 } 190 }
154 191
155 } // namespace net 192 } // namespace net
OLDNEW
« no previous file with comments | « net/base/cert_verifier.h ('k') | tools/valgrind/gtest_exclude/net_unittests.gtest_mac.txt » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698