Apply RAII to UnixSocket

include/iocore/eventsystem/UnixSocket.h

@@ -65,6 +65,14 @@ class UnixSocket
    */
   UnixSocket(int domain, int ctype, int protocol);
 
+  ~UnixSocket();
+
+  UnixSocket(UnixSocket const &other)            = delete;
+  UnixSocket &operator=(UnixSocket const &other) = delete;
+
+  UnixSocket(UnixSocket &&other);
+  UnixSocket &operator=(UnixSocket &&other);
+
   int get_fd() const;
 
   bool is_ok() const;

src/iocore/dns/DNS.cc

@@ -679,7 +679,7 @@ DNSHandler::retry_named(int ndx, ink_hrtime t, bool reopen)
     open_cons(&m_res->nsaddr_list[ndx].sa, true, ndx);
   }
   bool          over_tcp = dns_conn_mode == DNS_CONN_MODE::TCP_ONLY;
-  UnixSocket    con_sock = over_tcp ? tcpcon[ndx].sock : udpcon[ndx].sock;
+  UnixSocket   &con_sock = over_tcp ? tcpcon[ndx].sock : udpcon[ndx].sock;
   unsigned char buffer[MAX_DNS_REQUEST_LEN];
   Dbg(dbg_ctl_dns, "trying to resolve '%s' from DNS connection, ndx %d", try_server_names[try_servers], ndx);
   int r       = _ink_res_mkquery(m_res, try_server_names[try_servers], T_A, buffer, over_tcp);
@@ -703,7 +703,7 @@ DNSHandler::try_primary_named(bool reopen)
   if ((t - last_primary_retry) > DNS_PRIMARY_RETRY_PERIOD) {
     unsigned char buffer[MAX_DNS_REQUEST_LEN];
     bool          over_tcp = dns_conn_mode == DNS_CONN_MODE::TCP_ONLY;
-    UnixSocket    con_sock = over_tcp ? tcpcon[0].sock : udpcon[0].sock;
+    UnixSocket   &con_sock = over_tcp ? tcpcon[0].sock : udpcon[0].sock;
     last_primary_retry     = t;
     Dbg(dbg_ctl_dns, "trying to resolve '%s' from primary DNS connection", try_server_names[try_servers]);
     int r = _ink_res_mkquery(m_res, try_server_names[try_servers], T_A, buffer, over_tcp);
@@ -1183,7 +1183,7 @@ write_dns_event(DNSHandler *h, DNSEntry *e, bool over_tcp)
     h->release_query_id(e->id[dns_retries - e->retries]);
   }
   e->id[dns_retries - e->retries] = i;
-  UnixSocket con_sock             = over_tcp ? h->tcpcon[h->name_server].sock : h->udpcon[h->name_server].sock;
+  UnixSocket &con_sock            = over_tcp ? h->tcpcon[h->name_server].sock : h->udpcon[h->name_server].sock;
   Dbg(dbg_ctl_dns, "send query (qtype=%d) for %s to fd %d", e->qtype, e->qname, con_sock.get_fd());
 
   int s = con_sock.send(buffer, r, 0);

src/iocore/eventsystem/UnixSocket.cc

@@ -23,6 +23,7 @@
 
 #include "iocore/eventsystem/UnixSocket.h"
 
+#include "tscore/Diags.h"
 #include "tscore/ink_apidefs.h"
 #include "tscore/ink_config.h"
 #include "tscore/TextBuffer.h"
@@ -50,6 +51,27 @@ static int accept4(int sockfd, struct sockaddr *addr, socklen_t *addrlen, int fl
 #endif
 static unsigned int read_uint_from_fd(int fd);
 
+UnixSocket::~UnixSocket()
+{
+  if (this->is_ok()) {
+    Warning("Dropped UnixSocket without closing socket.\n");
+  }
+}
+
+UnixSocket::UnixSocket(UnixSocket &&other)
+{
+  this->fd = other.fd;
+  other.fd = NO_SOCK;
+}
+
+UnixSocket &
+UnixSocket::operator=(UnixSocket &&other)
+{
+  this->fd = other.fd;
+  other.fd = NO_SOCK;
+  return *this;
+}
+
 int
 UnixSocket::set_nonblocking()
 {

src/iocore/net/Connection.cc

@@ -72,9 +72,7 @@ Connection::move(Connection &orig)
 {
   this->is_connected = orig.is_connected;
   this->is_bound     = orig.is_bound;
-  this->sock         = orig.sock;
-  // The target has taken ownership of the file descriptor
-  orig.sock       = UnixSocket{NO_FD};
-  this->addr      = orig.addr;
-  this->sock_type = orig.sock_type;
+  this->sock         = std::move(orig.sock);
+  this->addr         = orig.addr;
+  this->sock_type    = orig.sock_type;
 }

src/iocore/net/NetAcceptEventIO.cc

@@ -28,7 +28,7 @@ int
 NetAcceptEventIO::start(EventLoop l, NetAccept *na, int events)
 {
   _na = na;
-  return start_common(l, _na->server.sock.get_fd(), events);
+  return start_common(l, _na->server->sock.get_fd(), events);
 }
 void
 NetAcceptEventIO::process_event(int /* flags ATS_UNUSED */)

src/iocore/net/P_Connection.h

@@ -109,7 +109,8 @@ struct Connection {
 
   virtual ~Connection();
   Connection();
-  Connection(Connection const &that) = delete;
+  Connection(Connection const &that)            = delete;
+  Connection &operator=(Connection const &that) = delete;
 
   /// Default options.
   static NetVCOptions const DEFAULT_OPTIONS;
@@ -119,15 +120,6 @@ struct Connection {
    */
   void move(Connection &);
 
-protected:
-  /** Assignment operator.
-   *
-   * @param that Source object.
-   * @return @a this
-   *
-   * This is protected because it is not safe in the general case, but is valid for
-   * certain subclasses. Those provide a public assignment that depends on this method.
-   */
-  Connection &operator=(Connection const &that) = default;
-  void        _cleanup();
+private:
+  void _cleanup();
 };

src/iocore/net/P_NetAccept.h

@@ -42,9 +42,11 @@
 #include "iocore/net/NetAcceptEventIO.h"
 #include "Server.h"
 
-#include <vector>
 #include "tscore/ink_platform.h"
 
+#include <memory>
+#include <vector>
+
 struct NetAccept;
 struct HttpProxyPort;
 class Event;
@@ -62,7 +64,7 @@ class UnixNetVConnection;
 
 // TODO fix race between cancel accept and call back
 struct NetAcceptAction : public Action, public RefCountObjInHeap {
-  Server *server;
+  std::shared_ptr<Server> server;
 
   void
   cancel(Continuation *cont = nullptr) override
@@ -89,14 +91,14 @@ struct NetAcceptAction : public Action, public RefCountObjInHeap {
 // Handles accepting connections.
 //
 struct NetAccept : public Continuation {
-  ink_hrtime             period = 0;
-  Server                 server;
-  AcceptFunctionPtr      accept_fn = nullptr;
-  int                    ifd       = NO_FD;
-  int                    id        = -1;
-  Ptr<NetAcceptAction>   action_;
-  SSLNextProtocolAccept *snpa = nullptr;
-  NetAcceptEventIO       ep;
+  ink_hrtime              period = 0;
+  std::shared_ptr<Server> server;
+  AcceptFunctionPtr       accept_fn = nullptr;
+  int                     ifd       = NO_FD;
+  int                     id        = -1;
+  Ptr<NetAcceptAction>    action_;
+  SSLNextProtocolAccept  *snpa = nullptr;
+  NetAcceptEventIO        ep;
 
   HttpProxyPort *proxyPort = nullptr;
   AcceptOptions  opt;

src/iocore/net/QUICNetProcessor.cc

@@ -250,13 +250,13 @@ QUICNetProcessor::main_accept(Continuation *cont, SOCKET fd, AcceptOptions const
   ink_assert(0 < opt.local_port && opt.local_port < 65536);
   accept_ip.network_order_port() = htons(opt.local_port);
 
-  na->accept_fn   = net_accept;
-  na->server.sock = UnixSocket{fd};
-  ats_ip_copy(&na->server.accept_addr, &accept_ip);
+  na->accept_fn    = net_accept;
+  na->server->sock = UnixSocket{fd};
+  ats_ip_copy(&na->server->accept_addr, &accept_ip);
 
   na->action_         = new NetAcceptAction();
   *na->action_        = cont;
-  na->action_->server = &na->server;
+  na->action_->server = na->server;
   na->init_accept();
 
   return na->action_.get();

src/iocore/net/QUICPacketHandler.cc

@@ -160,7 +160,7 @@ QUICPacketHandlerIn::acceptEvent(int event, void *data)
   } else if (event == EVENT_IMMEDIATE) {
     this->setThreadAffinity(this_ethread());
     SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
-    udpNet.UDPBind((Continuation *)this, &this->server.accept_addr.sa, -1, 1048576, 1048576);
+    udpNet.UDPBind((Continuation *)this, &this->server->accept_addr.sa, -1, 1048576, 1048576);
     return EVENT_CONT;
   }
 

src/iocore/net/Server.h

@@ -29,6 +29,8 @@
 #include "tscore/ink_inet.h"
 #include "tscore/ink_memory.h"
 
+#include <memory>
+
 struct Server {
   /// Client side (inbound) local IP address.
   IpEndpoint accept_addr;