Skip to content

Commit 3acf7e2

Browse files
committed
Drop the dummy no-std Condvar which never sleeps
In `no-std`, we exposed `wait` functions which rely on a dummy `Condvar` which never actually sleeps. This is somwhat nonsensical, not to mention confusing to users. Instead, we simply remove the `wait` methods in `no-std` builds.
1 parent efcb5e0 commit 3acf7e2

File tree

4 files changed

+35
-71
lines changed

4 files changed

+35
-71
lines changed

lightning/src/ln/channelmanager.rs

+16-15
Original file line numberDiff line numberDiff line change
@@ -7906,6 +7906,7 @@ mod tests {
79067906
use bitcoin::hashes::Hash;
79077907
use bitcoin::hashes::sha256::Hash as Sha256;
79087908
use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
7909+
#[cfg(feature = "std")]
79097910
use core::time::Duration;
79107911
use core::sync::atomic::Ordering;
79117912
use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
@@ -7931,9 +7932,9 @@ mod tests {
79317932

79327933
// All nodes start with a persistable update pending as `create_network` connects each node
79337934
// with all other nodes to make most tests simpler.
7934-
assert!(nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7935-
assert!(nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7936-
assert!(nodes[2].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7935+
assert!(nodes[0].node.get_persistable_update_future().poll_is_complete());
7936+
assert!(nodes[1].node.get_persistable_update_future().poll_is_complete());
7937+
assert!(nodes[2].node.get_persistable_update_future().poll_is_complete());
79377938

79387939
let mut chan = create_announced_chan_between_nodes(&nodes, 0, 1);
79397940

@@ -7947,28 +7948,28 @@ mod tests {
79477948
&nodes[0].node.get_our_node_id()).pop().unwrap();
79487949

79497950
// The first two nodes (which opened a channel) should now require fresh persistence
7950-
assert!(nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7951-
assert!(nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7951+
assert!(nodes[0].node.get_persistable_update_future().poll_is_complete());
7952+
assert!(nodes[1].node.get_persistable_update_future().poll_is_complete());
79527953
// ... but the last node should not.
7953-
assert!(!nodes[2].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7954+
assert!(!nodes[2].node.get_persistable_update_future().poll_is_complete());
79547955
// After persisting the first two nodes they should no longer need fresh persistence.
7955-
assert!(!nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7956-
assert!(!nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7956+
assert!(!nodes[0].node.get_persistable_update_future().poll_is_complete());
7957+
assert!(!nodes[1].node.get_persistable_update_future().poll_is_complete());
79577958

79587959
// Node 3, unrelated to the only channel, shouldn't care if it receives a channel_update
79597960
// about the channel.
79607961
nodes[2].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &chan.0);
79617962
nodes[2].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &chan.1);
7962-
assert!(!nodes[2].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7963+
assert!(!nodes[2].node.get_persistable_update_future().poll_is_complete());
79637964

79647965
// The nodes which are a party to the channel should also ignore messages from unrelated
79657966
// parties.
79667967
nodes[0].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.0);
79677968
nodes[0].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.1);
79687969
nodes[1].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.0);
79697970
nodes[1].node.handle_channel_update(&nodes[2].node.get_our_node_id(), &chan.1);
7970-
assert!(!nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7971-
assert!(!nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7971+
assert!(!nodes[0].node.get_persistable_update_future().poll_is_complete());
7972+
assert!(!nodes[1].node.get_persistable_update_future().poll_is_complete());
79727973

79737974
// At this point the channel info given by peers should still be the same.
79747975
assert_eq!(nodes[0].node.list_channels()[0], node_a_chan_info);
@@ -7985,17 +7986,17 @@ mod tests {
79857986
// persisted and that its channel info remains the same.
79867987
nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &as_update);
79877988
nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &bs_update);
7988-
assert!(!nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7989-
assert!(!nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7989+
assert!(!nodes[0].node.get_persistable_update_future().poll_is_complete());
7990+
assert!(!nodes[1].node.get_persistable_update_future().poll_is_complete());
79907991
assert_eq!(nodes[0].node.list_channels()[0], node_a_chan_info);
79917992
assert_eq!(nodes[1].node.list_channels()[0], node_b_chan_info);
79927993

79937994
// Finally, deliver the other peers' message, ensuring each node needs to be persisted and
79947995
// the channel info has updated.
79957996
nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &bs_update);
79967997
nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &as_update);
7997-
assert!(nodes[0].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7998-
assert!(nodes[1].node.get_persistable_update_future().wait_timeout(Duration::from_millis(1)));
7998+
assert!(nodes[0].node.get_persistable_update_future().poll_is_complete());
7999+
assert!(nodes[1].node.get_persistable_update_future().poll_is_complete());
79998000
assert_ne!(nodes[0].node.list_channels()[0], node_a_chan_info);
80008001
assert_ne!(nodes[1].node.list_channels()[0], node_b_chan_info);
80018002
}

lightning/src/sync/debug_sync.rs

-11
Original file line numberDiff line numberDiff line change
@@ -37,24 +37,13 @@ impl Condvar {
3737
Condvar { inner: StdCondvar::new() }
3838
}
3939

40-
pub fn wait<'a, T>(&'a self, guard: MutexGuard<'a, T>) -> LockResult<MutexGuard<'a, T>> {
41-
let mutex: &'a Mutex<T> = guard.mutex;
42-
self.inner.wait(guard.into_inner()).map(|lock| MutexGuard { mutex, lock }).map_err(|_| ())
43-
}
44-
4540
pub fn wait_while<'a, T, F: FnMut(&mut T) -> bool>(&'a self, guard: MutexGuard<'a, T>, condition: F)
4641
-> LockResult<MutexGuard<'a, T>> {
4742
let mutex: &'a Mutex<T> = guard.mutex;
4843
self.inner.wait_while(guard.into_inner(), condition).map(|lock| MutexGuard { mutex, lock })
4944
.map_err(|_| ())
5045
}
5146

52-
#[allow(unused)]
53-
pub fn wait_timeout<'a, T>(&'a self, guard: MutexGuard<'a, T>, dur: Duration) -> LockResult<(MutexGuard<'a, T>, ())> {
54-
let mutex = guard.mutex;
55-
self.inner.wait_timeout(guard.into_inner(), dur).map(|(lock, _)| (MutexGuard { mutex, lock }, ())).map_err(|_| ())
56-
}
57-
5847
#[allow(unused)]
5948
pub fn wait_timeout_while<'a, T, F: FnMut(&mut T) -> bool>(&'a self, guard: MutexGuard<'a, T>, dur: Duration, condition: F)
6049
-> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)> {

lightning/src/sync/nostd_sync.rs

-41
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,10 @@
11
pub use ::alloc::sync::Arc;
22
use core::ops::{Deref, DerefMut};
3-
use core::time::Duration;
43
use core::cell::{RefCell, Ref, RefMut};
54
use super::{LockTestExt, LockHeldState};
65

76
pub type LockResult<Guard> = Result<Guard, ()>;
87

9-
pub struct Condvar {}
10-
11-
pub struct WaitTimeoutResult(bool);
12-
impl WaitTimeoutResult {
13-
pub fn timed_out(&self) -> bool { self.0 }
14-
}
15-
16-
impl Condvar {
17-
pub fn new() -> Condvar {
18-
Condvar { }
19-
}
20-
21-
pub fn wait<'a, T>(&'a self, guard: MutexGuard<'a, T>) -> LockResult<MutexGuard<'a, T>> {
22-
Ok(guard)
23-
}
24-
25-
#[allow(unused)]
26-
pub fn wait_timeout<'a, T>(&'a self, guard: MutexGuard<'a, T>, _dur: Duration) -> LockResult<(MutexGuard<'a, T>, ())> {
27-
Ok((guard, ()))
28-
}
29-
30-
pub fn wait_while<'a, T, F: FnMut(&mut T) -> bool>(&'a self, mut guard: MutexGuard<'a, T>, mut condition: F)
31-
-> LockResult<MutexGuard<'a, T>> {
32-
assert!(!condition(&mut *guard));
33-
Ok(guard)
34-
}
35-
36-
#[allow(unused)]
37-
pub fn wait_timeout_while<'a, T, F: FnMut(&mut T) -> bool>(&'a self, mut guard: MutexGuard<'a, T>, dur: Duration, mut condition: F)
38-
-> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)> {
39-
if condition(&mut *guard) {
40-
Ok((guard, WaitTimeoutResult(true)))
41-
} else {
42-
Ok((guard, WaitTimeoutResult(false)))
43-
}
44-
}
45-
46-
pub fn notify_all(&self) {}
47-
}
48-
498
pub struct Mutex<T: ?Sized> {
509
inner: RefCell<T>
5110
}

lightning/src/util/wakers.rs

+19-4
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@
1515
1616
use alloc::sync::Arc;
1717
use core::mem;
18-
use crate::sync::{Condvar, Mutex};
18+
use crate::sync::Mutex;
1919

2020
use crate::prelude::*;
2121

22-
#[cfg(any(test, feature = "std"))]
22+
#[cfg(feature = "std")]
23+
use crate::sync::Condvar;
24+
#[cfg(feature = "std")]
2325
use std::time::Duration;
2426

2527
use core::future::Future as StdFuture;
@@ -160,17 +162,27 @@ impl Future {
160162
}
161163

162164
/// Waits until this [`Future`] completes.
165+
#[cfg(feature = "std")]
163166
pub fn wait(self) {
164167
Sleeper::from_single_future(self).wait();
165168
}
166169

167170
/// Waits until this [`Future`] completes or the given amount of time has elapsed.
168171
///
169172
/// Returns true if the [`Future`] completed, false if the time elapsed.
170-
#[cfg(any(test, feature = "std"))]
173+
#[cfg(feature = "std")]
171174
pub fn wait_timeout(self, max_wait: Duration) -> bool {
172175
Sleeper::from_single_future(self).wait_timeout(max_wait)
173176
}
177+
178+
#[cfg(test)]
179+
pub fn poll_is_complete(&self) -> bool {
180+
let mut state = self.state.lock().unwrap();
181+
if state.complete {
182+
state.callbacks_made = true;
183+
true
184+
} else { false }
185+
}
174186
}
175187

176188
use core::task::Waker;
@@ -198,10 +210,12 @@ impl<'a> StdFuture for Future {
198210

199211
/// A struct which can be used to select across many [`Future`]s at once without relying on a full
200212
/// async context.
213+
#[cfg(feature = "std")]
201214
pub struct Sleeper {
202215
notifiers: Vec<Arc<Mutex<FutureState>>>,
203216
}
204217

218+
#[cfg(feature = "std")]
205219
impl Sleeper {
206220
/// Constructs a new sleeper from one future, allowing blocking on it.
207221
pub fn from_single_future(future: Future) -> Self {
@@ -252,7 +266,6 @@ impl Sleeper {
252266
/// Wait until one of the [`Future`]s registered with this [`Sleeper`] has completed or the
253267
/// given amount of time has elapsed. Returns true if a [`Future`] completed, false if the time
254268
/// elapsed.
255-
#[cfg(any(test, feature = "std"))]
256269
pub fn wait_timeout(&self, max_wait: Duration) -> bool {
257270
let (cv, notified_fut_mtx) = self.setup_wait();
258271
let notified_fut =
@@ -478,6 +491,7 @@ mod tests {
478491
}
479492

480493
#[test]
494+
#[cfg(feature = "std")]
481495
fn test_dropped_future_doesnt_count() {
482496
// Tests that if a Future gets drop'd before it is poll()ed `Ready` it doesn't count as
483497
// having been woken, leaving the notify-required flag set.
@@ -569,6 +583,7 @@ mod tests {
569583
}
570584

571585
#[test]
586+
#[cfg(feature = "std")]
572587
fn test_multi_future_sleep() {
573588
// Tests the `Sleeper` with multiple futures.
574589
let notifier_a = Notifier::new();

0 commit comments

Comments
 (0)