Call DelFd() and SetFd(-1) from SocketEngine::Close(EventHandler*)

8 files changed, 15 insertions, 28 deletions

diff --git a/include/socketengine.h b/include/socketengine.h
index 305d389d4..3a15e98c1 100644
--- a/include/socketengine.h
+++ b/include/socketengine.h
@@ -409,12 +409,11 @@ public:
 	 */
 	static int Accept(EventHandler* fd, sockaddr *addr, socklen_t *addrlen);
 
-	/** Abstraction for BSD sockets close(2).
-	 * This function should emulate its namesake system call exactly.
-	 * @param fd This version of the call takes an EventHandler instead of a bare file descriptor.
-	 * @return This method should return exactly the same values as the system call it emulates.
+	/** Close the underlying fd of an event handler, remove it from the socket engine and set the fd to -1.
+	 * @param eh The EventHandler to close.
+	 * @return 0 on success, a negative value on error
 	 */
-	static int Close(EventHandler* fd);
+	static int Close(EventHandler* eh);
 
 	/** Abstraction for BSD sockets close(2).
 	 * This function should emulate its namesake system call exactly.
diff --git a/src/commands/cmd_dns.cpp b/src/commands/cmd_dns.cpp
index 63924d87b..7b93c7bb3 100644
--- a/src/commands/cmd_dns.cpp
+++ b/src/commands/cmd_dns.cpp
@@ -681,10 +681,8 @@ class MyManager : public Manager, public Timer, public EventHandler
 	{
 		if (this->GetFd() > -1)
 		{
-			SocketEngine::DelFd(this);
 			SocketEngine::Shutdown(this, 2);
 			SocketEngine::Close(this);
-			this->SetFd(-1);
 
 			/* Remove expired entries from the cache */
 			this->Tick(ServerInstance->Time());
@@ -710,13 +708,13 @@ class MyManager : public Manager, public Timer, public EventHandler
 			{
 				/* Failed to bind */
 				ServerInstance->Logs->Log("RESOLVER", LOG_SPARSE, "Resolver: Error binding dns socket - hostnames will NOT resolve");
-				SocketEngine::Close(this);
+				SocketEngine::Close(this->GetFd());
 				this->SetFd(-1);
 			}
 			else if (!SocketEngine::AddFd(this, FD_WANT_POLL_READ | FD_WANT_NO_WRITE))
 			{
 				ServerInstance->Logs->Log("RESOLVER", LOG_SPARSE, "Resolver: Internal error starting DNS - hostnames will NOT resolve.");
-				SocketEngine::Close(this);
+				SocketEngine::Close(this->GetFd());
 				this->SetFd(-1);
 			}
 		}
diff --git a/src/inspsocket.cpp b/src/inspsocket.cpp
index 3610addba..d15a1b6a5 100644
--- a/src/inspsocket.cpp
+++ b/src/inspsocket.cpp
@@ -138,9 +138,7 @@ void StreamSocket::Close()
 			DelIOHook();
 		}
 		SocketEngine::Shutdown(this, 2);
-		SocketEngine::DelFd(this);
 		SocketEngine::Close(this);
-		fd = -1;
 	}
 }
 
diff --git a/src/listensocket.cpp b/src/listensocket.cpp
index 0f926aaf6..e73a8f4ce 100644
--- a/src/listensocket.cpp
+++ b/src/listensocket.cpp
@@ -78,7 +78,7 @@ ListenSocket::ListenSocket(ConfigTag* tag, const irc::sockets::sockaddrs& bind_t
 	{
 		int errstore = errno;
 		SocketEngine::Shutdown(this, 2);
-		SocketEngine::Close(this);
+		SocketEngine::Close(this->GetFd());
 		this->fd = -1;
 		errno = errstore;
 	}
@@ -95,12 +95,10 @@ ListenSocket::~ListenSocket()
 {
 	if (this->GetFd() > -1)
 	{
-		SocketEngine::DelFd(this);
 		ServerInstance->Logs->Log("SOCKET", LOG_DEBUG, "Shut down listener on fd %d", this->fd);
 		SocketEngine::Shutdown(this, 2);
 		if (SocketEngine::Close(this) != 0)
 			ServerInstance->Logs->Log("SOCKET", LOG_DEBUG, "Failed to cancel listener: %s", strerror(errno));
-		this->fd = -1;
 	}
 }
 
diff --git a/src/modules/m_ident.cpp b/src/modules/m_ident.cpp
index bea4c2803..67065bd62 100644
--- a/src/modules/m_ident.cpp
+++ b/src/modules/m_ident.cpp
@@ -195,9 +195,7 @@ class IdentRequestSocket : public EventHandler
 		if (GetFd() > -1)
 		{
 			ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "Close ident socket %d", GetFd());
-			SocketEngine::DelFd(this);
-			SocketEngine::Close(GetFd());
-			this->SetFd(-1);
+			SocketEngine::Close(this);
 		}
 	}
 
diff --git a/src/modules/m_spanningtree/override_squit.cpp b/src/modules/m_spanningtree/override_squit.cpp
index 2bae80927..84cb01f50 100644
--- a/src/modules/m_spanningtree/override_squit.cpp
+++ b/src/modules/m_spanningtree/override_squit.cpp
@@ -42,7 +42,6 @@ ModResult ModuleSpanningTree::HandleSquit(const std::vector<std::string>& parame
 		{
 			ServerInstance->SNO->WriteToSnoMask('l',"SQUIT: Server \002%s\002 removed from network by %s",parameters[0].c_str(),user->nick.c_str());
 			sock->Squit(s,"Server quit by " + user->GetFullRealHost());
-			SocketEngine::DelFd(sock);
 			sock->Close();
 		}
 		else
diff --git a/src/socketengine.cpp b/src/socketengine.cpp
index 698b9061f..86608f0a3 100644
--- a/src/socketengine.cpp
+++ b/src/socketengine.cpp
@@ -147,13 +147,12 @@ int SocketEngine::Accept(EventHandler* fd, sockaddr *addr, socklen_t *addrlen)
 	return accept(fd->GetFd(), addr, addrlen);
 }
 
-int SocketEngine::Close(EventHandler* fd)
+int SocketEngine::Close(EventHandler* eh)
 {
-#ifdef _WIN32
-	return closesocket(fd->GetFd());
-#else
-	return close(fd->GetFd());
-#endif
+	DelFd(eh);
+	int ret = Close(eh->GetFd());
+	eh->SetFd(-1);
+	return ret;
 }
 
 int SocketEngine::Close(int fd)
diff --git a/src/threadengines/threadengine_pthread.cpp b/src/threadengines/threadengine_pthread.cpp
index 27641f0ac..8527907c4 100644
--- a/src/threadengines/threadengine_pthread.cpp
+++ b/src/threadengines/threadengine_pthread.cpp
@@ -80,8 +80,7 @@ class ThreadSignalSocket : public EventHandler
 
 	~ThreadSignalSocket()
 	{
-		SocketEngine::DelFd(this);
-		SocketEngine::Close(GetFd());
+		SocketEngine::Close(this);
 	}
 
 	void Notify()
@@ -130,8 +129,7 @@ class ThreadSignalSocket : public EventHandler
 	~ThreadSignalSocket()
 	{
 		close(send_fd);
-		SocketEngine::DelFd(this);
-		SocketEngine::Close(GetFd());
+		SocketEngine::Close(this);
 	}
 
 	void Notify()