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 <mbonadei@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34722}

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") {

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) {
+AsyncSocketAdapter::AsyncSocketAdapter(AsyncSocket* socket)
-  Attach(socket);
+    : socket_(absl::WrapUnique(socket)) {
-}
+  RTC_DCHECK(socket_);
-
+  socket_->SignalConnectEvent.connect(this,
-AsyncSocketAdapter::~AsyncSocketAdapter() {
+                                      &AsyncSocketAdapter::OnConnectEvent);
-  delete socket_;
+  socket_->SignalReadEvent.connect(this, &AsyncSocketAdapter::OnReadEvent);
-}
+  socket_->SignalWriteEvent.connect(this, &AsyncSocketAdapter::OnWriteEvent);
-
+  socket_->SignalCloseEvent.connect(this, &AsyncSocketAdapter::OnCloseEvent);
-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);
-  }
 }
 
 SocketAddress AsyncSocketAdapter::GetLocalAddress() const {

rtc_base/async_socket.h

@@ -14,6 +14,8 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <memory>
+
 #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.
+  // Takes ownership of the passed in socket.
-  // However, subclasses which support detached mode must override any methods
+  // TODO(bugs.webrtc.org/6424): Change to unique_ptr here and in callers.
-  // that will be called during the detached period (usually GetState()), to
-  // avoid dereferencing a null pointer.
   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<AsyncSocket> socket_;
 };
 
 }  // namespace rtc

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, decltype(&::BIO_free)> bio{BIO_new_socket(socket_),
+  std::unique_ptr<BIO, decltype(&::BIO_free)> 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 &&
+  if (GetSocket()->GetState() == Socket::CS_CONNECTED &&
-      addr == socket_->GetRemoteAddress()) {
+      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;

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 =
+  int len = AsyncSocketAdapter::Recv(buffer_ + data_len_,
-      socket_->Recv(buffer_ + data_len_, buffer_size_ - data_len_, nullptr);
+                                     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);