mirror of
https://github.com/outbackdingo/firezone.git
synced 2026-01-27 10:18:54 +00:00
chore(relay): be more lenient with debug-assertions (#5367)
Some of the debug-assertions in the relay are a bit too strict. Specifically, if an allocation times out because it is not refreshed, we also clean-up all channel bindings associated with that allocation. Yet, if an existing channel binding has already been removed earlier, it will no longer be present in the respective map. This isn't an issue at all. We can simply change the debug-assertion to only compare what used to be present in the map. What really matters is that the item we just removed does in fact point to the data that we are expecting. Related: #5355.
This commit is contained in:
@@ -406,8 +406,13 @@ where
|
||||
tracing::info!(target: "relay", channel = %number.value(), %client, peer = %channel.peer_address, allocation = %channel.allocation, "Channel is now expired");
|
||||
|
||||
channel.bound = false;
|
||||
self.channel_and_client_by_port_and_peer
|
||||
.remove(&(channel.allocation, channel.peer_address));
|
||||
if let Some((cs, n)) = self
|
||||
.channel_and_client_by_port_and_peer
|
||||
.remove(&(channel.allocation, channel.peer_address))
|
||||
{
|
||||
debug_assert_eq!(&cs, client, "internal state should be consistent");
|
||||
debug_assert_eq!(&n, number, "internal state should be consistent");
|
||||
};
|
||||
}
|
||||
|
||||
let channels_to_delete = self
|
||||
@@ -604,7 +609,7 @@ where
|
||||
) -> Result<(), Message<Attribute>> {
|
||||
self.verify_auth(&request)?;
|
||||
|
||||
let allocation = self
|
||||
let allocation: &mut Allocation = self
|
||||
.allocations
|
||||
.get_mut(&sender)
|
||||
.ok_or(error_response(AllocationMismatch, &request))?;
|
||||
@@ -887,28 +892,29 @@ where
|
||||
let port = allocation.port;
|
||||
|
||||
self.channels_by_client_and_number
|
||||
.retain(|(cl, number), c| {
|
||||
.retain(|(cs, number), c| {
|
||||
if c.allocation != port {
|
||||
return true;
|
||||
}
|
||||
|
||||
debug_assert_eq!(cl, &client, "internal state to be consistent");
|
||||
debug_assert_eq!(cs, &client, "internal state should be consistent");
|
||||
|
||||
let peer = c.peer_address;
|
||||
|
||||
let existing = self
|
||||
if let Some(existing) = self
|
||||
.channel_numbers_by_client_and_peer
|
||||
.remove(&(client, peer));
|
||||
debug_assert_eq!(existing, Some(*number), "internal state to be consistent");
|
||||
.remove(&(client, peer))
|
||||
{
|
||||
debug_assert_eq!(existing, *number, "internal state should be consistent");
|
||||
}
|
||||
|
||||
let existing = self
|
||||
if let Some((existing_cs, existing_n)) = self
|
||||
.channel_and_client_by_port_and_peer
|
||||
.remove(&(port, peer));
|
||||
debug_assert_eq!(
|
||||
existing,
|
||||
Some((client, *number)),
|
||||
"internal state to be consistent"
|
||||
);
|
||||
.remove(&(port, peer))
|
||||
{
|
||||
debug_assert_eq!(&existing_cs, cs, "internal state should be consistent");
|
||||
debug_assert_eq!(&existing_n, number, "internal state should be consistent");
|
||||
}
|
||||
|
||||
tracing::info!(%peer, %number, "Deleted channel binding");
|
||||
|
||||
@@ -938,14 +944,12 @@ where
|
||||
let peer = channel.peer_address;
|
||||
let allocation = channel.allocation;
|
||||
|
||||
let _peer_channel = self
|
||||
if let Some(_peer_channel) = self
|
||||
.channel_numbers_by_client_and_peer
|
||||
.remove(&(client, peer));
|
||||
debug_assert_eq!(
|
||||
_peer_channel,
|
||||
Some(chan),
|
||||
"Channel state should be consistent"
|
||||
);
|
||||
.remove(&(client, peer))
|
||||
{
|
||||
debug_assert_eq!(_peer_channel, chan, "internal state should be consistent");
|
||||
}
|
||||
|
||||
self.channels_by_client_and_number.remove(&(client, chan));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user