From 8729d785dfb7439c64eb90610db7478a917c1d59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Wed, 11 Aug 2021 11:22:44 +0200 Subject: [PATCH] Delete AsyncSocketAdapter::Attach, make socket construction time const Bug: webrtc:6424 Change-Id: I7001c4ac52ddd267dcd55f751f3f38976eab820f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227032 Reviewed-by: Mirko Bonadei Reviewed-by: Harald Alvestrand Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#34722} --- rtc_base/BUILD.gn | 1 + rtc_base/async_socket.cc | 27 +++++++++------------------ rtc_base/async_socket.h | 16 +++++++++------- rtc_base/openssl_adapter.cc | 12 ++++++------ rtc_base/socket_adapters.cc | 12 ++++++------ 5 files changed, 31 insertions(+), 37 deletions(-) diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 8dc89fafba..40c4835456 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -835,6 +835,7 @@ rtc_library("async_socket") { ":socket_address", "third_party/sigslot", ] + absl_deps = [ "//third_party/abseil-cpp/absl/memory" ] } rtc_library("socket") { diff --git a/rtc_base/async_socket.cc b/rtc_base/async_socket.cc index 90e2c2af9e..e80514db12 100644 --- a/rtc_base/async_socket.cc +++ b/rtc_base/async_socket.cc @@ -10,6 +10,7 @@ #include "rtc_base/async_socket.h" +#include "absl/memory/memory.h" #include "rtc_base/checks.h" namespace rtc { @@ -18,24 +19,14 @@ AsyncSocket::AsyncSocket() {} AsyncSocket::~AsyncSocket() {} -AsyncSocketAdapter::AsyncSocketAdapter(AsyncSocket* socket) : socket_(nullptr) { - Attach(socket); -} - -AsyncSocketAdapter::~AsyncSocketAdapter() { - delete socket_; -} - -void AsyncSocketAdapter::Attach(AsyncSocket* socket) { - RTC_DCHECK(!socket_); - socket_ = socket; - if (socket_) { - socket_->SignalConnectEvent.connect(this, - &AsyncSocketAdapter::OnConnectEvent); - socket_->SignalReadEvent.connect(this, &AsyncSocketAdapter::OnReadEvent); - socket_->SignalWriteEvent.connect(this, &AsyncSocketAdapter::OnWriteEvent); - socket_->SignalCloseEvent.connect(this, &AsyncSocketAdapter::OnCloseEvent); - } +AsyncSocketAdapter::AsyncSocketAdapter(AsyncSocket* socket) + : socket_(absl::WrapUnique(socket)) { + RTC_DCHECK(socket_); + socket_->SignalConnectEvent.connect(this, + &AsyncSocketAdapter::OnConnectEvent); + socket_->SignalReadEvent.connect(this, &AsyncSocketAdapter::OnReadEvent); + socket_->SignalWriteEvent.connect(this, &AsyncSocketAdapter::OnWriteEvent); + socket_->SignalCloseEvent.connect(this, &AsyncSocketAdapter::OnCloseEvent); } SocketAddress AsyncSocketAdapter::GetLocalAddress() const { diff --git a/rtc_base/async_socket.h b/rtc_base/async_socket.h index 7b2f0e0de9..9dc236fd12 100644 --- a/rtc_base/async_socket.h +++ b/rtc_base/async_socket.h @@ -14,6 +14,8 @@ #include #include +#include + #include "rtc_base/socket.h" #include "rtc_base/socket_address.h" #include "rtc_base/third_party/sigslot/sigslot.h" @@ -45,13 +47,10 @@ class AsyncSocket : public Socket { class AsyncSocketAdapter : public AsyncSocket, public sigslot::has_slots<> { public: - // The adapted socket may explicitly be null, and later assigned using Attach. - // However, subclasses which support detached mode must override any methods - // that will be called during the detached period (usually GetState()), to - // avoid dereferencing a null pointer. + // Takes ownership of the passed in socket. + // TODO(bugs.webrtc.org/6424): Change to unique_ptr here and in callers. explicit AsyncSocketAdapter(AsyncSocket* socket); - ~AsyncSocketAdapter() override; - void Attach(AsyncSocket* socket); + SocketAddress GetLocalAddress() const override; SocketAddress GetRemoteAddress() const override; int Bind(const SocketAddress& addr) override; @@ -78,7 +77,10 @@ class AsyncSocketAdapter : public AsyncSocket, public sigslot::has_slots<> { virtual void OnWriteEvent(AsyncSocket* socket); virtual void OnCloseEvent(AsyncSocket* socket, int err); - AsyncSocket* socket_; + AsyncSocket* GetSocket() const { return socket_.get(); } + + private: + const std::unique_ptr socket_; }; } // namespace rtc diff --git a/rtc_base/openssl_adapter.cc b/rtc_base/openssl_adapter.cc index 42195de6fd..d6e1a60d5f 100644 --- a/rtc_base/openssl_adapter.cc +++ b/rtc_base/openssl_adapter.cc @@ -271,7 +271,7 @@ int OpenSSLAdapter::StartSSL(const char* hostname) { ssl_host_name_ = hostname; - if (socket_->GetState() != Socket::CS_CONNECTED) { + if (GetSocket()->GetState() != Socket::CS_CONNECTED) { state_ = SSL_WAIT; return 0; } @@ -308,7 +308,7 @@ int OpenSSLAdapter::BeginSSL() { return -1; } - std::unique_ptr bio{BIO_new_socket(socket_), + std::unique_ptr bio{BIO_new_socket(GetSocket()), ::BIO_free}; if (!bio) { return -1; @@ -578,8 +578,8 @@ int OpenSSLAdapter::Send(const void* pv, size_t cb) { int OpenSSLAdapter::SendTo(const void* pv, size_t cb, const SocketAddress& addr) { - if (socket_->GetState() == Socket::CS_CONNECTED && - addr == socket_->GetRemoteAddress()) { + if (GetSocket()->GetState() == Socket::CS_CONNECTED && + addr == GetSocket()->GetRemoteAddress()) { return Send(pv, cb); } @@ -640,7 +640,7 @@ int OpenSSLAdapter::RecvFrom(void* pv, size_t cb, SocketAddress* paddr, int64_t* timestamp) { - if (socket_->GetState() == Socket::CS_CONNECTED) { + if (GetSocket()->GetState() == Socket::CS_CONNECTED) { int ret = Recv(pv, cb, timestamp); *paddr = GetRemoteAddress(); return ret; @@ -657,7 +657,7 @@ int OpenSSLAdapter::Close() { } Socket::ConnState OpenSSLAdapter::GetState() const { - ConnState state = socket_->GetState(); + ConnState state = GetSocket()->GetState(); if ((state == CS_CONNECTED) && ((state_ == SSL_WAIT) || (state_ == SSL_CONNECTING))) { state = CS_CONNECTING; diff --git a/rtc_base/socket_adapters.cc b/rtc_base/socket_adapters.cc index dc9d883d58..ee20a76e32 100644 --- a/rtc_base/socket_adapters.cc +++ b/rtc_base/socket_adapters.cc @@ -41,7 +41,7 @@ BufferedReadAdapter::~BufferedReadAdapter() { int BufferedReadAdapter::Send(const void* pv, size_t cb) { if (buffering_) { // TODO: Spoof error better; Signal Writeable - socket_->SetError(EWOULDBLOCK); + SetError(EWOULDBLOCK); return -1; } return AsyncSocketAdapter::Send(pv, cb); @@ -49,7 +49,7 @@ int BufferedReadAdapter::Send(const void* pv, size_t cb) { int BufferedReadAdapter::Recv(void* pv, size_t cb, int64_t* timestamp) { if (buffering_) { - socket_->SetError(EWOULDBLOCK); + SetError(EWOULDBLOCK); return -1; } @@ -88,7 +88,7 @@ void BufferedReadAdapter::BufferInput(bool on) { } void BufferedReadAdapter::OnReadEvent(AsyncSocket* socket) { - RTC_DCHECK(socket == socket_); + RTC_DCHECK(socket == GetSocket()); if (!buffering_) { AsyncSocketAdapter::OnReadEvent(socket); @@ -101,8 +101,8 @@ void BufferedReadAdapter::OnReadEvent(AsyncSocket* socket) { data_len_ = 0; } - int len = - socket_->Recv(buffer_ + data_len_, buffer_size_ - data_len_, nullptr); + int len = AsyncSocketAdapter::Recv(buffer_ + data_len_, + buffer_size_ - data_len_, nullptr); if (len < 0) { // TODO: Do something better like forwarding the error to the user. RTC_LOG_ERR(INFO) << "Recv"; @@ -179,7 +179,7 @@ int AsyncSSLSocket::Connect(const SocketAddress& addr) { } void AsyncSSLSocket::OnConnectEvent(AsyncSocket* socket) { - RTC_DCHECK(socket == socket_); + RTC_DCHECK(socket == GetSocket()); // TODO: we could buffer output too... const int res = DirectSend(kSslClientHello, sizeof(kSslClientHello)); RTC_DCHECK_EQ(sizeof(kSslClientHello), res);