Skip to content

Commit 4fd1149

Browse files
committed
Fix warnings for redundant closed connections
Use a close code for a redundant conn closure so that we can detect it for the purpose of identifying and logging the close properly.
1 parent 7ec68a3 commit 4fd1149

File tree

4 files changed

+29
-14
lines changed

4 files changed

+29
-14
lines changed

src/link/connection.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ namespace srouter::link
1010
: conn{std::move(c)}, datagrams{conn->datagrams()}, control_stream{std::move(s)}
1111
{}
1212

13-
void Connection::close()
13+
void Connection::close(uint64_t errcode)
1414
{
1515
log::trace(logcat, "{} called", __PRETTY_FUNCTION__);
16-
conn->close_connection();
16+
conn->close_connection(errcode);
1717
}
1818
} // namespace srouter::link

src/link/connection.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@ namespace srouter::link
2424

2525
bool is_inbound() const { return conn->is_inbound(); }
2626

27-
void close();
27+
void close(uint64_t errcode = 0);
2828
};
2929
} // namespace srouter::link

src/link/endpoint.cpp

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,31 +27,31 @@ namespace srouter::link
2727
conn = ptr.get();
2828
}
2929

30-
void relay_conn::close(bool direction_inbound)
30+
void relay_conn::close(bool direction_inbound, uint64_t errcode)
3131
{
3232
auto& to_close = direction_inbound ? inbound : outbound;
3333
if (not to_close)
3434
return;
3535

36-
to_close->close();
36+
to_close->close(errcode);
3737
to_close.reset();
3838

3939
// Switch preferred conn to the other direction (will be nullptr if the other direction
4040
// isn't established):
4141
conn = (direction_inbound ? outbound : inbound).get();
4242
}
4343

44-
void relay_conn::close_all()
44+
void relay_conn::close_all(uint64_t errcode)
4545
{
4646
if (inbound)
47-
inbound->close();
47+
inbound->close(errcode);
4848
if (outbound)
49-
outbound->close();
49+
outbound->close(errcode);
5050
inbound = outbound = nullptr;
5151
conn = nullptr;
5252
}
5353

54-
void relay_conn::close_redundant() { close(not inbound_wins); }
54+
void relay_conn::close_redundant() { close(not inbound_wins, CONN_CLOSE_REDUNDANT); }
5555

5656
static std::vector<uint8_t> make_static_secret(
5757
const Ed25519SecretKey& sk, std::string_view static_secret_key = "Session Router static shared secret key"sv)
@@ -275,7 +275,8 @@ namespace srouter::link
275275
it = pending_dead.erase(it);
276276
}
277277

278-
// Record timestamps for any newly unregistered nodes
278+
// Record timestamps for any newly unregistered nodes so that, when the timer expires, we
279+
// drop the connection.
279280
for (const auto& [rid, rconn] : relay_conns)
280281
if (!registered.contains(rid) && pending_dead.emplace(rid, now).second)
281282
log::critical(
@@ -867,6 +868,18 @@ namespace srouter::link
867868
pending_outbound.erase(it);
868869
found = true;
869870
}
871+
872+
if (!found && ec == CONN_CLOSE_REDUNDANT)
873+
{
874+
log::debug(
875+
logcat,
876+
"Closed redundant connection {} {} @ {} (cid={})",
877+
conn.is_inbound() ? "from" : "to",
878+
rid->to_network_address(true),
879+
conn.remote(),
880+
conn.reference_id());
881+
found = true;
882+
}
870883
}
871884
else if (alpn == CLIENT_ALPN)
872885
{
@@ -916,9 +929,9 @@ namespace srouter::link
916929
if (!found)
917930
log::warning(
918931
logcat,
919-
"Closed untracked connection {} {} @ {} (cid={}, ec={})",
932+
"Closed connection {} {} @ {} (cid={}, ec={})",
920933
conn.is_inbound() ? "from" : "to",
921-
rid ? rid->to_network_address(true /* don't know! */).to_string() : "",
934+
rid ? rid->to_string() : "",
922935
conn.remote(),
923936
conn.reference_id(),
924937
ec);

src/link/endpoint.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ namespace srouter::link
3939
// longer than the longest path to allow those clients to naturally migrate to new paths.
4040
inline constexpr auto DEREGGED_LINGER = 30min;
4141

42+
static constexpr uint64_t CONN_CLOSE_REDUNDANT = 6;
43+
4244
// Stores relay-to-relay connections. In order to not lose stream messages, we temporarily
4345
// allow simultaneous connections in both directions between a pair of relays, but then
4446
// after a timeout, both sides choose the same winner and drop the other one. The timeout
@@ -67,10 +69,10 @@ namespace srouter::link
6769
// Closes either the inbound or outbound connection and drops it from this instance. If
6870
// the other connection still exists then `conn` is updated to point at it, otherwise it
6971
// is set to nullptr. Does nothing if the indicated connection is already closed.
70-
void close(bool direction_inbound);
72+
void close(bool direction_inbound, uint64_t errcode = 0);
7173

7274
// Closes all connections, in both directions (if opened).
73-
void close_all();
75+
void close_all(uint64_t errcode = 0);
7476

7577
// Closes the "loser" connection, if this instance has connections in both directions.
7678
void close_redundant();

0 commit comments

Comments
 (0)