Skip to content

Commit

Permalink
Merge pull request #58 from embassy-rs/disconnect-busyloop
Browse files Browse the repository at this point in the history
Fix disconnect busyloop
  • Loading branch information
lulf authored Jun 3, 2024
2 parents 7878ce9 + 506eac5 commit bb51ed1
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 9 deletions.
2 changes: 1 addition & 1 deletion host/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ log = { version = "0.4.16", optional = true }
defmt = {version = "0.3", optional = true }

[dev-dependencies]
tokio = { version = "1", features = ["full"] }
tokio = { version = "1", default-features = false, features = ["time", "rt-multi-thread", "macros"] }
embedded-io-adapters = { version = "0.6.1", features = ["tokio-1"] }
embedded-io-async = { version = "0.6.1" }
tokio-serial = "5.4"
Expand Down
61 changes: 53 additions & 8 deletions host/src/connection_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,10 @@ impl<'d> ConnectionManager<'d> {

pub(crate) fn request_disconnect(&self, index: u8, reason: DisconnectReason) {
self.with_mut(|state| {
let state = &mut state.connections[index as usize];
if state.state == ConnectionState::Connected {
state.state = ConnectionState::DisconnectRequest(reason);
let entry = &mut state.connections[index as usize];
if entry.state == ConnectionState::Connected {
entry.state = ConnectionState::DisconnectRequest(reason);
state.disconnect_waker.wake();
}
})
}
Expand All @@ -86,10 +87,10 @@ impl<'d> ConnectionManager<'d> {
if let Some(cx) = cx {
state.disconnect_waker.register(cx.waker());
}
for storage in state.connections.iter() {
for (idx, storage) in state.connections.iter().enumerate() {
if let ConnectionState::DisconnectRequest(_) = storage.state {
return Poll::Ready(DisconnectRequestIter {
next: 0,
next: idx,
state: &self.state,
});
}
Expand All @@ -116,7 +117,6 @@ impl<'d> ConnectionManager<'d> {
if let Some(handle) = storage.handle {
if handle == h && storage.state != ConnectionState::Disconnected {
storage.state = ConnectionState::Disconnected;
state.disconnect_waker.wake();
return Ok(());
}
}
Expand Down Expand Up @@ -224,8 +224,6 @@ impl<'d> ConnectionManager<'d> {
);
if state.refcount == 0 && state.state == ConnectionState::Connected {
state.state = ConnectionState::DisconnectRequest(DisconnectReason::RemoteUserTerminatedConn);
// ensure it is never reused
state.handle.take();
}
});
}
Expand Down Expand Up @@ -580,6 +578,14 @@ mod tests {
let next = unwrap!(it.next());
assert!(it.next().is_none());

// If nothing happens, polling should behave the same way
let Poll::Ready(mut it) = mgr.poll_disconnecting(None) else {
panic!("expected connection to be accepted");
};

let next = unwrap!(it.next());
assert!(it.next().is_none());

// Disconnection event from host arrives before we confirm
unwrap!(mgr.disconnected(ConnHandle::new(2)));

Expand Down Expand Up @@ -717,4 +723,43 @@ mod tests {

assert!(!conn2.is_connected());
}

#[test]
fn disconnecting_iterator_invalid() {
let mut storage = [ConnectionStorage::DISCONNECTED; 3];
let mgr = ConnectionManager::new(&mut storage[..]);

assert!(mgr.poll_accept(LeConnRole::Peripheral, &[], None).is_pending());

unwrap!(mgr.connect(
ConnHandle::new(3),
AddrKind::RANDOM,
BdAddr::new(ADDR_1),
LeConnRole::Peripheral
));

let Poll::Ready(handle) = mgr.poll_accept(LeConnRole::Peripheral, &[], None) else {
panic!("expected connection to be accepted");
};
assert_eq!(handle.role(), LeConnRole::Peripheral);
assert_eq!(handle.peer_address(), BdAddr::new(ADDR_1));

assert!(mgr.poll_disconnecting(None).is_pending());

// Disconnect request from us
drop(handle);

// Polling should return the disconnecting handle
let Poll::Ready(mut it) = mgr.poll_disconnecting(None) else {
panic!("expected connection to be accepted");
};

// unwrap!(mgr.disconnected(ConnHandle::new(3)));

let n = it.next();
assert!(n.is_some());
n.map(|n| n.confirm());

assert!(mgr.poll_disconnecting(None).is_pending());
}
}

0 comments on commit bb51ed1

Please sign in to comment.