client: fix double-close condition for connections

Currently, to avoid double-close of a connection we check for the `SS_DEAD`
status on its stream. However, the `SS_DEAD` status can be set for a
multitude of reasons without the connection being closed or even connected.
For instance, we set it if a send/recv failed unrecoverably.

To fix this, let's rely on the fact that a connection is open iff the file
descriptor of its stream is a valid (i.e., not -1). This approach seems to
be portable to the Windows platform too.

Closes #142

Author: CuriousGeorgiy

src/Client/Connection.hpp

@@ -125,7 +125,7 @@ template<class BUFFER, class NetProvider>
 ConnectionImpl<BUFFER, NetProvider>::~ConnectionImpl()
 {
 	assert(refs == 0);
-	if (!strm.has_status(SS_DEAD)) {
+	if (strm.is_open()) {
 		connector.close(this);
 	}
 }

src/Client/Connector.hpp

@@ -182,14 +182,15 @@ Connector<BUFFER, NetProvider>::close(Connection<BUFFER, NetProvider> &conn)
 	return close(conn.getImpl());
 }
 
-template<class BUFFER, class NetProvider>
+template <class BUFFER, class NetProvider>
 void
 Connector<BUFFER, NetProvider>::close(ConnectionImpl<BUFFER, NetProvider> *conn)
 {
-	assert(!conn->get_strm().has_status(SS_DEAD));
-	m_NetProvider.close(conn->get_strm());
-	m_ReadyToSend.erase(conn);
-	m_ReadyToDecode.erase(conn);
+	if (conn->get_strm().is_open()) {
+		m_NetProvider.close(conn->get_strm());
+		m_ReadyToSend.erase(conn);
+		m_ReadyToDecode.erase(conn);
+	}
 }
 
 template <class BUFFER, class NetProvider>

src/Client/UnixStream.hpp

@@ -66,6 +66,9 @@ class UnixStream : public Stream {
 	/** Get internal file descriptor of the socket. */
 	int get_fd() const { return fd; }
 
+	/** Check whether the socket is open. */
+	bool is_open() const { return fd != -1; }
+
 protected:
 	/** Log helpers. */
 	template <class... MSG>

test/ClientTest.cpp

@@ -182,6 +182,8 @@ trivial(Connector<BUFFER, NetProvider> &client)
 	TEST_CASE("Connect timeout");
 	rc = test_connect(client, conn, "8.8.8.8", port);
 	fail_unless(rc != 0);
+	TEST_CASE("Close of non-established connection (gh-142)");
+	client.close(conn);
 }
 
 /** Single connection, separate/sequence pings, no errors */
@@ -235,6 +237,9 @@ single_conn_ping(Connector<BUFFER, NetProvider> &client)
 		fail_unless(response->body.error_stack == std::nullopt);
 	}
 	client.close(conn);
+
+	TEST_CASE("Double close of connection (gh-142)");
+	client.close(conn);
 }
 
 template <class BUFFER, class NetProvider>
@@ -1082,6 +1087,10 @@ test_sigpipe(void)
 	fail_unless(saved_errno == EPIPE);
 #endif
 	fail_if(conn.futureIsReady(f));
+
+	TEST_CASE("Close of connection with error (gh-142)");
+	fail_unless(conn.hasError());
+	client.close(conn);
 }
 
 /** Single connection, wait response from closed connection. */