From 7d3d2ed0aa4396620c38aee1aafd2e56f6133b5e Mon Sep 17 00:00:00 2001 From: "rsleevi@chromium.org" Date: Tue, 10 Jul 2012 22:56:23 +0000 Subject: Switch the NSS thread from being a base::Thread to a base::SequencedWorkerPool of 1 BUG=135435 TEST=existing + tsan bots Review URL: https://chromiumcodereview.appspot.com/10749009 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@145977 0039d316-1c4b-4281-b951-d872f2087c98 --- net/socket/client_socket_factory.cc | 17 ++++++++++------- net/socket/ssl_client_socket_nss.cc | 8 ++++---- net/socket/ssl_client_socket_nss.h | 6 +++--- tools/valgrind/tsan/suppressions.txt | 7 ------- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/net/socket/client_socket_factory.cc b/net/socket/client_socket_factory.cc index f507f4e..b5b9ca6 100644 --- a/net/socket/client_socket_factory.cc +++ b/net/socket/client_socket_factory.cc @@ -6,7 +6,7 @@ #include "base/lazy_instance.h" #include "base/thread_task_runner_handle.h" -#include "base/threading/thread.h" +#include "base/threading/sequenced_worker_pool.h" #include "build/build_config.h" #include "net/base/cert_database.h" #include "net/socket/client_socket_handle.h" @@ -47,9 +47,12 @@ class DefaultClientSocketFactory : public ClientSocketFactory, public: DefaultClientSocketFactory() { if (g_use_dedicated_nss_thread) { - nss_thread_.reset(new base::Thread("NSS SSL Thread")); - if (nss_thread_->Start()) - nss_thread_task_runner_ = nss_thread_->message_loop_proxy(); + // Use a single thread for the worker pool. + worker_pool_ = new base::SequencedWorkerPool(1, "NSS SSL Thread"); + nss_thread_task_runner_ = + worker_pool_->GetSequencedTaskRunnerWithShutdownBehavior( + worker_pool_->GetSequenceToken(), + base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN); } CertDatabase::AddObserver(this); @@ -101,7 +104,7 @@ class DefaultClientSocketFactory : public ClientSocketFactory, // between each test. Because the DefaultClientSocketFactory is leaky, it // may span multiple tests, and thus the current task runner may change // from call to call. - scoped_refptr nss_task_runner( + scoped_refptr nss_task_runner( nss_thread_task_runner_); if (!nss_task_runner) nss_task_runner = base::ThreadTaskRunnerHandle::Get(); @@ -139,8 +142,8 @@ class DefaultClientSocketFactory : public ClientSocketFactory, } private: - scoped_ptr nss_thread_; - scoped_refptr nss_thread_task_runner_; + scoped_refptr worker_pool_; + scoped_refptr nss_thread_task_runner_; }; static base::LazyInstance::Leaky diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc index 8d677ef..0c3ea4e 100644 --- a/net/socket/ssl_client_socket_nss.cc +++ b/net/socket/ssl_client_socket_nss.cc @@ -707,7 +707,7 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe { // |server_bound_cert_service|, and they will not be accessed once Detach() // has been called. Core(base::SequencedTaskRunner* network_task_runner, - base::SingleThreadTaskRunner* nss_task_runner, + base::SequencedTaskRunner* nss_task_runner, ClientSocketHandle* transport, const HostPortPair& host_and_port, const SSLConfig& ssl_config, @@ -977,7 +977,7 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe { // task runner. //////////////////////////////////////////////////////////////////////////// scoped_refptr network_task_runner_; - scoped_refptr nss_task_runner_; + scoped_refptr nss_task_runner_; // Dereferenced only on the network task runner, but bound to tasks destined // for the network task runner from the NSS task runner. @@ -996,7 +996,7 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe { SSLClientSocketNSS::Core::Core( base::SequencedTaskRunner* network_task_runner, - base::SingleThreadTaskRunner* nss_task_runner, + base::SequencedTaskRunner* nss_task_runner, ClientSocketHandle* transport, const HostPortPair& host_and_port, const SSLConfig& ssl_config, @@ -2711,7 +2711,7 @@ void SSLClientSocketNSS::Core::SetChannelIDProvided() { } SSLClientSocketNSS::SSLClientSocketNSS( - base::SingleThreadTaskRunner* nss_task_runner, + base::SequencedTaskRunner* nss_task_runner, ClientSocketHandle* transport_socket, const HostPortPair& host_and_port, const SSLConfig& ssl_config, diff --git a/net/socket/ssl_client_socket_nss.h b/net/socket/ssl_client_socket_nss.h index 204b1cb..12c9186 100644 --- a/net/socket/ssl_client_socket_nss.h +++ b/net/socket/ssl_client_socket_nss.h @@ -31,7 +31,7 @@ #include "net/socket/ssl_client_socket.h" namespace base { -class SingleThreadTaskRunner; +class SequencedTaskRunner; } namespace net { @@ -59,7 +59,7 @@ class SSLClientSocketNSS : public SSLClientSocket { // NSS may optionally be run on a dedicated thread. If synchronous/blocking // behaviour is desired, for performance or compatibility, the current task // runner should be supplied instead. - SSLClientSocketNSS(base::SingleThreadTaskRunner* nss_task_runner, + SSLClientSocketNSS(base::SequencedTaskRunner* nss_task_runner, ClientSocketHandle* transport_socket, const HostPortPair& host_and_port, const SSLConfig& ssl_config, @@ -145,7 +145,7 @@ class SSLClientSocketNSS : public SSLClientSocket { bool CalledOnValidThread() const; // The task runner used to perform NSS operations. - scoped_refptr nss_task_runner_; + scoped_refptr nss_task_runner_; scoped_ptr transport_; HostPortPair host_and_port_; SSLConfig ssl_config_; diff --git a/tools/valgrind/tsan/suppressions.txt b/tools/valgrind/tsan/suppressions.txt index 855e577..effc886 100644 --- a/tools/valgrind/tsan/suppressions.txt +++ b/tools/valgrind/tsan/suppressions.txt @@ -985,10 +985,3 @@ fun:media::PipelineIntegrationTestBase::OnVideoRendererPaint fun:base::internal::RunnableAdapter::Run } -{ - bug_135435 - ThreadSanitizer:Race - fun:base::subtle::NoBarrier_Store - fun:base::LazyInstance::OnExit - fun:base::internal::RunnableAdapter::Run -} -- cgit v1.1