don't require absense of cancel handles

Prior to this change, we required that services that are canceled do not
have a cancel handle in the `cancel_handles` list, based on the
assumption that the handle must have been removed in the process of
canceling this service.

This doesn't holding up though, because it is currently possible for us
to have the same peer connect to us multiple times, the second connect
removes the cancel handle of the original connect and inserts it's own
cancel handle in its place. In this scenario, when the first service is
polled for readiness it will see that it has been canceled and go to
clean itself up, but when it asserts that it doesn't have a cancel
handle it will see the cancel handle of the second connect event, which
uses the same key as the first connect, and fail its debug assertion.

This change removes that debug assert on the assumption that it is okay
for a peer to connect multiple times consecutively, and that the correct
behavior in that case is to just cancel the first connection and
continue as normal.
This commit is contained in:
Jane Lusby 2020-06-16 13:12:09 -07:00 committed by Henry de Valence
parent 06fd3b2503
commit 685bdaf2df
1 changed files with 8 additions and 4 deletions

View File

@ -152,7 +152,7 @@ where
Some(j) => Some(j), // We swapped an unrelated service.
};
// No Heisenservices: they must be ready or unready.
debug_assert!(!self.cancel_handles.contains_key(key));
assert!(!self.cancel_handles.contains_key(key));
} else if let Some(handle) = self.cancel_handles.remove(key) {
let _ = handle.send(());
}
@ -200,18 +200,22 @@ where
Poll::Ready(Some(Ok((key, svc)))) => {
trace!(?key, "service became ready");
let _cancel = self.cancel_handles.remove(&key);
debug_assert!(_cancel.is_some(), "missing cancel handle");
assert!(_cancel.is_some(), "missing cancel handle");
self.ready_services.insert(key, svc);
}
Poll::Ready(Some(Err((key, UnreadyError::Canceled)))) => {
trace!(?key, "service was canceled");
debug_assert!(!self.cancel_handles.contains_key(&key))
// This debug assert is invalid because we can have a
// service be canceled due us connecting to the same service
// twice.
//
// assert!(!self.cancel_handles.contains_key(&key))
}
Poll::Ready(Some(Err((key, UnreadyError::Inner(e))))) => {
let error = e.into();
debug!(%error, "service failed while unready, dropped");
let _cancel = self.cancel_handles.remove(&key);
debug_assert!(_cancel.is_some(), "missing cancel handle");
assert!(_cancel.is_some(), "missing cancel handle");
}
}
}