diff options
author | hclam@chromium.org <hclam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-02 22:38:10 +0000 |
---|---|---|
committer | hclam@chromium.org <hclam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-02 22:38:10 +0000 |
commit | d04c6c85ad3b373b7eb6e29516f4b3c35b1158d1 (patch) | |
tree | ae4d3710edf730e67c59c746b02697c4d1358936 /remoting | |
parent | b35a36b143155169426362f5c9b22a2568ed303b (diff) | |
download | chromium_src-d04c6c85ad3b373b7eb6e29516f4b3c35b1158d1.zip chromium_src-d04c6c85ad3b373b7eb6e29516f4b3c35b1158d1.tar.gz chromium_src-d04c6c85ad3b373b7eb6e29516f4b3c35b1158d1.tar.bz2 |
SocketWrapper for interfacing between JingleSession and SSL sockets
Remoting code allows sockets to be destroyed on threads other than the network
thread, although they are only accessed on network thread. If SSL sockets
are destroyed too early some remotings objects will crash accessing them.
This patch introduces a SocketWrapper that destroys SSL sockets on the network
thread but stay alive so that remoting objects can operate safely.
BUG=72670
TEST=None
Review URL: http://codereview.chromium.org/6525007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76635 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'remoting')
-rw-r--r-- | remoting/host/chromoting_host.cc | 7 | ||||
-rw-r--r-- | remoting/protocol/jingle_session.cc | 17 | ||||
-rw-r--r-- | remoting/protocol/jingle_session.h | 16 | ||||
-rw-r--r-- | remoting/protocol/socket_wrapper.cc | 52 | ||||
-rw-r--r-- | remoting/protocol/socket_wrapper.h | 52 | ||||
-rw-r--r-- | remoting/remoting.gyp | 2 |
6 files changed, 129 insertions, 17 deletions
diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc index 56297dd..38437e8 100644 --- a/remoting/host/chromoting_host.cc +++ b/remoting/host/chromoting_host.cc @@ -127,6 +127,11 @@ void ChromotingHost::Shutdown() { state_ = kStopped; } + // Make sure ScreenRecorder doesn't write to the connection. + if (recorder_.get()) { + recorder_->RemoveAllConnections(); + } + // Disconnect the client. if (connection_) { connection_->Disconnect(); @@ -148,9 +153,7 @@ void ChromotingHost::Shutdown() { jingle_client_->Close(); } - // Tell the recorder to stop and then disconnect all clients. if (recorder_.get()) { - recorder_->RemoveAllConnections(); recorder_->Stop(shutdown_task_.release()); } else { shutdown_task_->Run(); diff --git a/remoting/protocol/jingle_session.cc b/remoting/protocol/jingle_session.cc index cf49d53..74ed354 100644 --- a/remoting/protocol/jingle_session.cc +++ b/remoting/protocol/jingle_session.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -19,6 +19,7 @@ #include "remoting/jingle_glue/jingle_thread.h" #include "remoting/jingle_glue/stream_socket_adapter.h" #include "remoting/protocol/jingle_session_manager.h" +#include "remoting/protocol/socket_wrapper.h" #include "third_party/libjingle/source/talk/base/thread.h" #include "third_party/libjingle/source/talk/p2p/base/session.h" #include "third_party/libjingle/source/talk/session/tunnel/pseudotcpchannel.h" @@ -125,7 +126,7 @@ void JingleSession::CloseInternal(int result, bool failed) { closing_ = true; if (control_ssl_socket_.get()) - control_ssl_socket_.reset(); + control_ssl_socket_->Disconnect(); if (control_channel_adapter_.get()) control_channel_adapter_->Close(result); @@ -136,7 +137,7 @@ void JingleSession::CloseInternal(int result, bool failed) { } if (event_ssl_socket_.get()) - event_ssl_socket_.reset(); + event_ssl_socket_->Disconnect(); if (event_channel_adapter_.get()) event_channel_adapter_->Close(result); @@ -147,7 +148,7 @@ void JingleSession::CloseInternal(int result, bool failed) { } if (video_ssl_socket_.get()) - video_ssl_socket_.reset(); + video_ssl_socket_->Disconnect(); if (video_channel_adapter_.get()) video_channel_adapter_->Close(result); @@ -393,13 +394,13 @@ void JingleSession::OnInitiate() { } bool JingleSession::EstablishSSLConnection( - net::ClientSocket* adapter, scoped_ptr<net::Socket>* ssl_socket) { + net::ClientSocket* adapter, scoped_ptr<SocketWrapper>* ssl_socket) { if (cricket_session_->initiator()) { // Create client SSL socket. net::SSLClientSocket* socket = CreateSSLClientSocket(adapter, server_cert_, cert_verifier_.get()); - ssl_socket->reset(socket); + ssl_socket->reset(new SocketWrapper(socket)); int ret = socket->Connect(connect_callback_.get()); if (ret == net::ERR_IO_PENDING) { @@ -414,7 +415,7 @@ bool JingleSession::EstablishSSLConnection( net::SSLConfig ssl_config; net::SSLServerSocket* socket = net::CreateSSLServerSocket( adapter, server_cert_, key_.get(), ssl_config); - ssl_socket->reset(socket); + ssl_socket->reset(new SocketWrapper(socket)); int ret = socket->Accept(connect_callback_.get()); if (ret == net::ERR_IO_PENDING) { @@ -422,7 +423,7 @@ bool JingleSession::EstablishSSLConnection( } else if (ret != net::OK) { LOG(ERROR) << "Failed to establish SSL connection"; cricket_session_->Terminate(); - return true; + return false; } } // Reach here if net::OK is received. diff --git a/remoting/protocol/jingle_session.h b/remoting/protocol/jingle_session.h index 617ded8..29aae81 100644 --- a/remoting/protocol/jingle_session.h +++ b/remoting/protocol/jingle_session.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -32,6 +32,7 @@ class TransportChannelSocketAdapter; namespace protocol { class JingleSessionManager; +class SocketWrapper; // Implements protocol::Session that work over libjingle session (the // cricket::Session object is passed to Init() method). Created @@ -101,7 +102,7 @@ class JingleSession : public protocol::Session, // provided. The resultant SSL socket is written to |ssl_socket|. // Return true if successful. bool EstablishSSLConnection(net::ClientSocket* adapter, - scoped_ptr<net::Socket>* ssl_socket); + scoped_ptr<SocketWrapper>* ssl_socket); // Used for Session.SignalState sigslot. void OnSessionState(cricket::BaseSession* session, @@ -150,19 +151,20 @@ class JingleSession : public protocol::Session, scoped_ptr<const CandidateSessionConfig> candidate_config_; // A channel is the the base channel created by libjingle. - // A channel adapter is used to convert a jingle channel to net::Socket. - // A SSL socket is a wrapper over a net::Socket to provide SSL functionality. + // A channel adapter is used to convert a jingle channel to net::Socket and + // then there is a SocketWrapper created over net::Socket. + // SSL socket uses SocketWrapper to provide SSL functionality. cricket::PseudoTcpChannel* control_channel_; scoped_ptr<StreamSocketAdapter> control_channel_adapter_; - scoped_ptr<net::Socket> control_ssl_socket_; + scoped_ptr<SocketWrapper> control_ssl_socket_; cricket::PseudoTcpChannel* event_channel_; scoped_ptr<StreamSocketAdapter> event_channel_adapter_; - scoped_ptr<net::Socket> event_ssl_socket_; + scoped_ptr<SocketWrapper> event_ssl_socket_; cricket::PseudoTcpChannel* video_channel_; scoped_ptr<StreamSocketAdapter> video_channel_adapter_; - scoped_ptr<net::Socket> video_ssl_socket_; + scoped_ptr<SocketWrapper> video_ssl_socket_; // Count the number of SSL connections esblished. int ssl_connections_; diff --git a/remoting/protocol/socket_wrapper.cc b/remoting/protocol/socket_wrapper.cc new file mode 100644 index 0000000..29b2bbf --- /dev/null +++ b/remoting/protocol/socket_wrapper.cc @@ -0,0 +1,52 @@ +// Copyright (c) 2011 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 "remoting/protocol/socket_wrapper.h" + +#include "base/logging.h" +#include "net/base/net_errors.h" + +namespace remoting { +namespace protocol { + +SocketWrapper::SocketWrapper(net::Socket* socket) + : socket_(socket) { +} + +SocketWrapper::~SocketWrapper() { + DCHECK(!socket_.get()); +} + +int SocketWrapper::Read(net::IOBuffer* buf, int buf_len, + net::CompletionCallback* callback) { + if (!socket_.get()) + return net::ERR_SOCKET_NOT_CONNECTED; + return socket_->Read(buf, buf_len, callback); +} + +int SocketWrapper::Write(net::IOBuffer* buf, int buf_len, + net::CompletionCallback* callback) { + if (!socket_.get()) + return net::ERR_SOCKET_NOT_CONNECTED; + return socket_->Write(buf, buf_len, callback); +} + +bool SocketWrapper::SetReceiveBufferSize(int32 size) { + if (!socket_.get()) + return false; + return socket_->SetReceiveBufferSize(size); +} + +bool SocketWrapper::SetSendBufferSize(int32 size) { + if (!socket_.get()) + return false; + return socket_->SetSendBufferSize(size); +} + +void SocketWrapper::Disconnect() { + socket_.reset(); +} + +} // namespace protocol +} // namespace remoting diff --git a/remoting/protocol/socket_wrapper.h b/remoting/protocol/socket_wrapper.h new file mode 100644 index 0000000..7176232 --- /dev/null +++ b/remoting/protocol/socket_wrapper.h @@ -0,0 +1,52 @@ +// Copyright (c) 2011 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. + +// This file should only be included by remoting/protocol/jingle_session.cc. + +#ifndef REMOTING_PROTOCOL_SOCKET_WRAPPER_H_ +#define REMOTING_PROTOCOL_SOCKET_WRAPPER_H_ + +#include "base/scoped_ptr.h" +#include "net/socket/socket.h" + +namespace remoting { +namespace protocol { + +// This class is used only to wrap over SSL sockets in JingleSession. +// There is a strong assumption in Chromium's code that sockets are used and +// destroyed on the same thread. However in remoting code we may destroy +// sockets on other thread. A wrapper is added between JingleSession and +// SSL sockets so we can destroy SSL sockets and still maintain valid +// references. +class SocketWrapper : public net::Socket { + public: + SocketWrapper(net::Socket* socket); + virtual ~SocketWrapper(); + + // net::Socket implementation. + virtual int Read(net::IOBuffer* buf, int buf_len, + net::CompletionCallback* callback); + virtual int Write(net::IOBuffer* buf, int buf_len, + net::CompletionCallback* callback); + virtual bool SetReceiveBufferSize(int32 size); + virtual bool SetSendBufferSize(int32 size); + + // Method to allow us to destroy the internal socket. This method must be + // called before SocketWrapper is destroyed. + // + // This method must be called on the same thread where this object is + // created. + void Disconnect(); + + private: + // The internal socket. + scoped_ptr<net::Socket> socket_; + + DISALLOW_COPY_AND_ASSIGN(SocketWrapper); +}; + +} // namespace protocol +} // namespace remoting + +#endif // REMOTING_PROTOCOL_SOCKET_WRAPPER_H_ diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp index 34f01a1..d57af12 100644 --- a/remoting/remoting.gyp +++ b/remoting/remoting.gyp @@ -420,6 +420,8 @@ 'protocol/session_manager.h', 'protocol/socket_reader_base.cc', 'protocol/socket_reader_base.h', + 'protocol/socket_wrapper.cc', + 'protocol/socket_wrapper.h', 'protocol/util.cc', 'protocol/util.h', 'protocol/video_reader.cc', |