Skip to content

Commit 1a5f36c

Browse files
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
1 parent 8d70145 commit 1a5f36c

File tree

3 files changed

+14
-4
lines changed

3 files changed

+14
-4
lines changed

src/Client/Connection.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ template<class BUFFER, class NetProvider>
124124
ConnectionImpl<BUFFER, NetProvider>::~ConnectionImpl()
125125
{
126126
assert(refs == 0);
127-
if (!strm.has_status(SS_DEAD)) {
127+
if (strm.get_fd() != -1) {
128128
connector.close(this);
129129
}
130130
}

src/Client/Connector.hpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,12 +184,13 @@ Connector<BUFFER, NetProvider>::close(Connection<BUFFER, NetProvider> &conn)
184184
return close(conn.getImpl());
185185
}
186186

187-
template<class BUFFER, class NetProvider>
187+
template <class BUFFER, class NetProvider>
188188
void
189189
Connector<BUFFER, NetProvider>::close(ConnectionImpl<BUFFER, NetProvider> *conn)
190190
{
191-
assert(!conn->get_strm().has_status(SS_DEAD));
192-
m_NetProvider.close(conn->get_strm());
191+
if (conn->get_strm().get_fd() != -1) {
192+
m_NetProvider.close(conn->get_strm());
193+
}
193194
}
194195

195196
template <class BUFFER, class NetProvider>

test/ClientTest.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@ trivial(Connector<BUFFER, NetProvider> &client)
182182
TEST_CASE("Connect timeout");
183183
rc = test_connect(client, conn, "8.8.8.8", port);
184184
fail_unless(rc != 0);
185+
TEST_CASE("Close of non-established connection (gh-142)");
186+
client.close(conn);
185187
}
186188

187189
/** Single connection, separate/sequence pings, no errors */
@@ -235,6 +237,9 @@ single_conn_ping(Connector<BUFFER, NetProvider> &client)
235237
fail_unless(response->body.error_stack == std::nullopt);
236238
}
237239
client.close(conn);
240+
241+
TEST_CASE("Double close of connection (gh-142)");
242+
client.close(conn);
238243
}
239244

240245
template <class BUFFER, class NetProvider>
@@ -1065,6 +1070,10 @@ test_sigpipe(void)
10651070
fail_unless(saved_errno == EPIPE);
10661071
#endif
10671072
fail_if(conn.futureIsReady(f));
1073+
1074+
TEST_CASE("Close of connection with error (gh-142)");
1075+
fail_unless(conn.hasError());
1076+
client.close(conn);
10681077
}
10691078

10701079
/** Single connection, wait response from closed connection. */

0 commit comments

Comments
 (0)