cpp/openttd-patchpack/source Changeset - r25458:3aaccd46b7fa

Changeset - r25458:3aaccd46b7fa

Parent rev.

Child rev.

[Not reviewed]

master

0 17 0

Patric Stout - 3 years ago 2021-05-13 09:46:51
truebrain@openttd.org

Codechange: [Network] split CloseSocket and CloseConnection more clearly (#9261)

* Codechange: [Network] split CloseSocket and CloseConnection more clearly

- CloseSocket now closes the actual OS socket.
- CloseConnection frees up the resources to just before CloseSocket.
- dtors call CloseSocket / CloseConnection where needed.

17 files changed with 78 insertions and 73 deletions:

src/network/core/core.h

src/network/core/packet.cpp

src/network/core/tcp.cpp

src/network/core/tcp.h

src/network/core/tcp_admin.cpp

src/network/core/tcp_admin.h

src/network/core/tcp_content.cpp

src/network/core/tcp_content.h

src/network/core/tcp_http.cpp

src/network/core/tcp_http.h

src/network/core/udp.cpp

src/network/core/udp.h

src/network/network_client.cpp

src/network/network_content.cpp

src/network/network_content.h

src/network/network_content_gui.cpp

src/network/network_udp.cpp

0 comments (0 inline, 0 general)

src/network/core/core.h

➞

Show inline comments

@@ @@ -40,7 +40,9 @@ struct Packet; @@
  * SocketHandler for all network sockets in OpenTTD.
  */
 class NetworkSocketHandler {
 private:
 	bool has_quit; ///< Whether the current client has quit/send a bad packet
 public:
 	/** Create a new unbound socket */
 	NetworkSocketHandler() { this->has_quit = false; }
@@ @@ -49,12 +51,13 @@ public: @@
 	virtual ~NetworkSocketHandler() {}
 	/**
 	 * Close the current connection; for TCP this will be mostly equivalent
 	 * to Close(), but for UDP it just means the packet has to be dropped.
 	 * @param error Whether we quit under an error condition or not.
 	 * @return new status of the connection.
 	 * Mark the connection as closed.
+	 *
 	 * This doesn't mean the actual connection is closed, but just that we
 	 * act like it is. This is useful for UDP, which doesn't normally close
 	 * a socket, but its handler might need to pretend it does.
 	 */
-	virtual NetworkRecvStatus CloseConnection(bool error = true) { this->has_quit = true; return NETWORK_RECV_STATUS_OKAY; }
+	void MarkClosed() { this->has_quit = true; }
 	/**
 	 * Whether the current client connected to the socket has quit.

src/network/core/packet.cpp

➞

Show inline comments

@@ @@ -222,7 +222,7 @@ bool Packet::CanReadFromPacket(size_t by @@
 	/* Check if variable is within packet-size */
 	if (this->pos + bytes_to_read > this->Size()) {
-		if (close_connection) this->cs->NetworkSocketHandler::CloseConnection();
+		if (close_connection) this->cs->NetworkSocketHandler::MarkClosed();
 		return false;
+	}

src/network/core/tcp.cpp

➞

Show inline comments

@@ @@ -29,24 +29,45 @@ NetworkTCPSocketHandler::NetworkTCPSocke @@
 NetworkTCPSocketHandler::~NetworkTCPSocketHandler()
+{
 	/* Virtual functions get called statically in destructors, so make it explicit to remove any confusion. */
 	this->NetworkTCPSocketHandler::CloseConnection();
 	if (this->sock != INVALID_SOCKET) closesocket(this->sock);
 	this->sock = INVALID_SOCKET;
 	this->EmptyPacketQueue();
 	this->CloseSocket();
+}
 NetworkRecvStatus NetworkTCPSocketHandler::CloseConnection(bool error)
 /**
  * Free all pending and partially received packets.
  */
 void NetworkTCPSocketHandler::EmptyPacketQueue()
+{
 	this->writable = false;
 	NetworkSocketHandler::CloseConnection(error);
 	/* Free all pending and partially received packets */
 	while (this->packet_queue != nullptr) {
 		delete Packet::PopFromQueue(&this->packet_queue);
+	}
 	delete this->packet_recv;
 	this->packet_recv = nullptr;
+}
 /**
  * Close the actual socket of the connection.
  * Please make sure CloseConnection is called before CloseSocket, as
  * otherwise not all resources might be released.
  */
 void NetworkTCPSocketHandler::CloseSocket()
+{
 	if (this->sock != INVALID_SOCKET) closesocket(this->sock);
 	this->sock = INVALID_SOCKET;
+}
 /**
  * This will put this socket handler in a close state. It will not
  * actually close the OS socket; use CloseSocket for this.
  * @param error Whether we quit under an error condition or not.
  * @return new status of the connection.
  */
 NetworkRecvStatus NetworkTCPSocketHandler::CloseConnection(bool error)
+{
 	this->MarkClosed();
 	this->writable = false;
 	this->EmptyPacketQueue();
 	return NETWORK_RECV_STATUS_OKAY;
+}

src/network/core/tcp.h

➞

Show inline comments

@@ @@ -33,6 +33,8 @@ class NetworkTCPSocketHandler : public N @@
 private:
 	Packet *packet_queue;     ///< Packets that are awaiting delivery
 	Packet *packet_recv;      ///< Partially received packet
 	void EmptyPacketQueue();
 public:
 	SOCKET sock;              ///< The socket currently connected to
 	bool writable;            ///< Can we write to this socket?
@@ @@ -43,7 +45,9 @@ public: @@
 	 */
 	bool IsConnected() const { return this->sock != INVALID_SOCKET; }
 	NetworkRecvStatus CloseConnection(bool error = true) override;
 	virtual NetworkRecvStatus CloseConnection(bool error = true);
 	void CloseSocket();
 	virtual void SendPacket(Packet *packet);
 	SendPacketsState SendPackets(bool closing_down = false);

src/network/core/tcp_admin.cpp

➞

Show inline comments

@@ @@ -34,10 +34,6 @@ NetworkAdminSocketHandler::NetworkAdminS @@
 	this->admin_version[0] = '\0';
+}
 NetworkAdminSocketHandler::~NetworkAdminSocketHandler()
+{
+}
 NetworkRecvStatus NetworkAdminSocketHandler::CloseConnection(bool error)
+{
 	delete this;

src/network/core/tcp_admin.h

➞

Show inline comments

@@ @@ -482,7 +482,6 @@ public: @@
 	NetworkRecvStatus CloseConnection(bool error = true) override;
 	NetworkAdminSocketHandler(SOCKET s);
 	~NetworkAdminSocketHandler();
 	NetworkRecvStatus ReceivePackets();

src/network/core/tcp_content.cpp

➞

Show inline comments

@@ @@ -138,17 +138,6 @@ const char *ContentInfo::GetTextfile(Tex @@
+}
 /**
  * Close the actual socket.
  */
 void NetworkContentSocketHandler::CloseSocket()
+{
 	if (this->sock == INVALID_SOCKET) return;
 	closesocket(this->sock);
 	this->sock = INVALID_SOCKET;
+}
 /**
  * Handle the given packet, i.e. pass it to the right
  * parser receive command.
  * @param p the packet to handle

src/network/core/tcp_content.h

➞

Show inline comments

@@ @@ -21,8 +21,6 @@ @@
 /** Base socket handler for all Content TCP sockets */
 class NetworkContentSocketHandler : public NetworkTCPSocketHandler {
 protected:
 	void CloseSocket();
 	bool ReceiveInvalidPacket(PacketContentType type);
 	/**

src/network/core/tcp_http.cpp

➞

Show inline comments

@@ @@ -68,17 +68,18 @@ NetworkHTTPSocketHandler::NetworkHTTPSoc @@
 /** Free whatever needs to be freed. */
 NetworkHTTPSocketHandler::~NetworkHTTPSocketHandler()
+{
-	this->CloseConnection();
+	this->CloseSocket();
 	if (this->sock != INVALID_SOCKET) closesocket(this->sock);
 	this->sock = INVALID_SOCKET;
 	free(this->data);
+}
 NetworkRecvStatus NetworkHTTPSocketHandler::CloseConnection(bool error)
 /**
  * Close the actual socket of the connection.
  */
 void NetworkHTTPSocketHandler::CloseSocket()
+{
 	NetworkSocketHandler::CloseConnection(error);
 	return NETWORK_RECV_STATUS_OKAY;
 	if (this->sock != INVALID_SOCKET) closesocket(this->sock);
 	this->sock = INVALID_SOCKET;
+}
 /**
@@ @@ -313,7 +314,7 @@ int NetworkHTTPSocketHandler::Receive() @@
 			if (ret < 0) cur->callback->OnFailure();
 			if (ret <= 0) {
 				/* Then... the connection can be closed */
-				cur->CloseConnection();
+				cur->CloseSocket();
 				iter = _http_connections.erase(iter);
 				delete cur;
 				continue;

src/network/core/tcp_http.h

➞

Show inline comments

@@ @@ -58,7 +58,7 @@ public: @@
 		return this->sock != INVALID_SOCKET;
+	}
-	NetworkRecvStatus CloseConnection(bool error = true) override;
+	void CloseSocket();
 	NetworkHTTPSocketHandler(SOCKET sock, HTTPCallback *callback,
 			const char *host, const char *url, const char *data, int depth);

src/network/core/udp.cpp

➞

Show inline comments

@@ @@ -44,7 +44,7 @@ NetworkUDPSocketHandler::NetworkUDPSocke @@
 bool NetworkUDPSocketHandler::Listen()
+{
 	/* Make sure socket is closed */
-	this->Close();
+	this->CloseSocket();
 	for (NetworkAddress &addr : this->bind) {
 		addr.Listen(SOCK_DGRAM, &this->sockets);
@@ @@ -54,9 +54,9 @@ bool NetworkUDPSocketHandler::Listen() @@
+}
 /**
- * Close the given UDP socket
+ * Close the actual UDP socket.
  */
-void NetworkUDPSocketHandler::Close()
+void NetworkUDPSocketHandler::CloseSocket()
+{
 	for (auto &s : this->sockets) {
 		closesocket(s.second);
@@ @@ -64,12 +64,6 @@ void NetworkUDPSocketHandler::Close() @@
 	this->sockets.clear();
+}
 NetworkRecvStatus NetworkUDPSocketHandler::CloseConnection(bool error)
+{
 	NetworkSocketHandler::CloseConnection(error);
 	return NETWORK_RECV_STATUS_OKAY;
+}
 /**
  * Send a packet over UDP
  * @param p    the packet to send

src/network/core/udp.h

➞

Show inline comments

@@ @@ -49,8 +49,6 @@ protected: @@
 	/** The opened sockets. */
 	SocketList sockets;
 	NetworkRecvStatus CloseConnection(bool error = true) override;
 	void ReceiveInvalidPacket(PacketUDPType, NetworkAddress *client_addr);
 	/**
@@ @@ -187,10 +185,10 @@ public: @@
 	NetworkUDPSocketHandler(NetworkAddressList *bind = nullptr);
 	/** On destructing of this class, the socket needs to be closed */
-	virtual ~NetworkUDPSocketHandler() { this->Close(); }
+	virtual ~NetworkUDPSocketHandler() { this->CloseSocket(); }
 	bool Listen();
-	void Close();
+	void CloseSocket();
 	void SendPacket(Packet *p, NetworkAddress *recv, bool all = false, bool broadcast = false);
 	void ReceivePackets();

src/network/network_client.cpp

➞

Show inline comments

@@ @@ -158,6 +158,7 @@ ClientNetworkGameSocketHandler::~ClientN @@
 	ClientNetworkGameSocketHandler::my_client = nullptr;
 	delete this->savegame;
 	delete this->GetInfo();
+}
 NetworkRecvStatus ClientNetworkGameSocketHandler::CloseConnection(NetworkRecvStatus status)
@@ @@ -182,7 +183,6 @@ NetworkRecvStatus ClientNetworkGameSocke @@
 	 * which would trigger the server to close the connection as well. */
 	CSleep(3 * MILLISECONDS_PER_TICK);
 	delete this->GetInfo();
 	delete this;
 	return status;
@@ @@ -200,7 +200,7 @@ void ClientNetworkGameSocketHandler::Cli @@
 	/* We just want to close the connection.. */
 	if (res == NETWORK_RECV_STATUS_CLOSE_QUERY) {
-		this->NetworkSocketHandler::CloseConnection();
+		this->NetworkSocketHandler::MarkClosed();
 		this->CloseConnection(res);
 		_networking = false;

src/network/network_content.cpp

➞

Show inline comments

@@ @@ -76,7 +76,7 @@ bool ClientNetworkContentSocketHandler:: @@
 	if (!ci->IsValid()) {
 		delete ci;
-		this->Close();
+		this->CloseConnection();
 		return false;
+	}
@@ @@ -488,7 +488,7 @@ bool ClientNetworkContentSocketHandler:: @@
 		p->Recv_string(this->curInfo->filename, lengthof(this->curInfo->filename));
 		if (!this->BeforeDownload()) {
-			this->Close();
+			this->CloseConnection();
 			return false;
+		}
 	} else {
@@ @@ -497,7 +497,7 @@ bool ClientNetworkContentSocketHandler:: @@
 		if (toRead != 0 && (size_t)p->TransferOut(TransferOutFWrite, this->curFile) != toRead) {
 			DeleteWindowById(WC_NETWORK_STATUS_WINDOW, WN_NETWORK_STATUS_WINDOW_CONTENT_DOWNLOAD);
 			ShowErrorMessage(STR_CONTENT_ERROR_COULD_NOT_DOWNLOAD, STR_CONTENT_ERROR_COULD_NOT_DOWNLOAD_FILE_NOT_WRITABLE, WL_ERROR);
-			this->Close();
+			this->CloseConnection();
 			fclose(this->curFile);
 			this->curFile = nullptr;
@@ @@ -781,14 +781,16 @@ void ClientNetworkContentSocketHandler:: @@
 /**
  * Disconnect from the content server.
  */
-void ClientNetworkContentSocketHandler::Close()
+NetworkRecvStatus ClientNetworkContentSocketHandler::CloseConnection(bool error)
+{
-	if (this->sock == INVALID_SOCKET) return;
+	NetworkContentSocketHandler::CloseConnection();
 	this->CloseConnection();
 	if (this->sock == INVALID_SOCKET) return NETWORK_RECV_STATUS_OKAY;
 	this->CloseSocket();
 	this->OnDisconnect();
-	this->OnDisconnect();
+	return NETWORK_RECV_STATUS_OKAY;
+}
 /**
@@ @@ -800,7 +802,7 @@ void ClientNetworkContentSocketHandler:: @@
 	if (this->sock == INVALID_SOCKET || this->isConnecting) return;
 	if (std::chrono::steady_clock::now() > this->lastActivity + IDLE_TIMEOUT) {
-		this->Close();
+		this->CloseConnection();
 		return;
+	}

src/network/network_content.h

➞

Show inline comments

@@ @@ -107,7 +107,7 @@ public: @@
 	void Connect();
 	void SendReceive();
-	void Close();
+	NetworkRecvStatus CloseConnection(bool error = true) override;
 	void RequestContentList(ContentType type);
 	void RequestContentList(uint count, const ContentID *content_ids);

src/network/network_content_gui.cpp

➞

Show inline comments

@@ @@ -260,7 +260,7 @@ public: @@
+	{
 		if (widget == WID_NCDS_CANCELOK) {
 			if (this->downloaded_bytes != this->total_bytes) {
-				_network_content_client.Close();
+				_network_content_client.CloseConnection();
 				delete this;
 			} else {
 				/* If downloading succeeded, close the online content window. This will close

src/network/network_udp.cpp

➞

Show inline comments

@@ @@ -54,10 +54,10 @@ struct UDPSocket { @@
 	UDPSocket(const std::string &name_) : name(name_), socket(nullptr) {}
-	void Close()
+	void CloseSocket()
+	{
 		std::lock_guard<std::mutex> lock(mutex);
-		socket->Close();
+		socket->CloseSocket();
 		delete socket;
 		socket = nullptr;
+	}
@@ @@ -619,9 +619,9 @@ void NetworkUDPServerListen() @@
 /** Close all UDP related stuff. */
 void NetworkUDPClose()
+{
 	_udp_client.Close();
 	_udp_server.Close();
 	_udp_master.Close();
 	_udp_client.CloseSocket();
 	_udp_server.CloseSocket();
 	_udp_master.CloseSocket();
 	_network_udp_server = false;
 	_network_udp_broadcast = 0;

0 comments (0 inline, 0 general)