Skip to content

Commit 1bc48ab

Browse files
committed
Switch PersistenceNotifierGuard persist closure to FnOnce
This allows us to move owned and mutable values into the closure, with the assumption that the closure can only be invoked once. As a result, we now have to track `should_persist` as an `Option`, such that we can `take` the owned closure and invoke it within the `PersistenceNotifierGuard::drop` implementation.
1 parent 217a5b0 commit 1bc48ab

File tree

1 file changed

+21
-11
lines changed

1 file changed

+21
-11
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2820,10 +2820,12 @@ enum NotifyOption {
28202820
/// We allow callers to either always notify by constructing with `notify_on_drop` or choose to
28212821
/// notify or not based on whether relevant changes have been made, providing a closure to
28222822
/// `optionally_notify` which returns a `NotifyOption`.
2823-
struct PersistenceNotifierGuard<'a, F: FnMut() -> NotifyOption> {
2823+
struct PersistenceNotifierGuard<'a, F: FnOnce() -> NotifyOption> {
28242824
event_persist_notifier: &'a Notifier,
28252825
needs_persist_flag: &'a AtomicBool,
2826-
should_persist: F,
2826+
// Always `Some` once initialized, but tracked as an `Option` to obtain the closure by value in
2827+
// [`PersistenceNotifierGuard::drop`].
2828+
should_persist: Option<F>,
28272829
// We hold onto this result so the lock doesn't get released immediately.
28282830
_read_guard: RwLockReadGuard<'a, ()>,
28292831
}
@@ -2838,20 +2840,20 @@ impl<'a> PersistenceNotifierGuard<'a, fn() -> NotifyOption> {
28382840
/// isn't ideal.
28392841
fn notify_on_drop<C: AChannelManager>(
28402842
cm: &'a C,
2841-
) -> PersistenceNotifierGuard<'a, impl FnMut() -> NotifyOption> {
2843+
) -> PersistenceNotifierGuard<'a, impl FnOnce() -> NotifyOption> {
28422844
Self::optionally_notify(cm, || -> NotifyOption { NotifyOption::DoPersist })
28432845
}
28442846

28452847
#[rustfmt::skip]
2846-
fn optionally_notify<F: FnMut() -> NotifyOption, C: AChannelManager>(cm: &'a C, mut persist_check: F)
2847-
-> PersistenceNotifierGuard<'a, impl FnMut() -> NotifyOption> {
2848+
fn optionally_notify<F: FnOnce() -> NotifyOption, C: AChannelManager>(cm: &'a C, persist_check: F)
2849+
-> PersistenceNotifierGuard<'a, impl FnOnce() -> NotifyOption> {
28482850
let read_guard = cm.get_cm().total_consistency_lock.read().unwrap();
28492851
let force_notify = cm.get_cm().process_background_events();
28502852

28512853
PersistenceNotifierGuard {
28522854
event_persist_notifier: &cm.get_cm().event_persist_notifier,
28532855
needs_persist_flag: &cm.get_cm().needs_persist_flag,
2854-
should_persist: move || {
2856+
should_persist: Some(move || {
28552857
// Pick the "most" action between `persist_check` and the background events
28562858
// processing and return that.
28572859
let notify = persist_check();
@@ -2862,7 +2864,7 @@ impl<'a> PersistenceNotifierGuard<'a, fn() -> NotifyOption> {
28622864
(_, NotifyOption::SkipPersistHandleEvents) => NotifyOption::SkipPersistHandleEvents,
28632865
_ => NotifyOption::SkipPersistNoEvents,
28642866
}
2865-
},
2867+
}),
28662868
_read_guard: read_guard,
28672869
}
28682870
}
@@ -2878,15 +2880,22 @@ impl<'a> PersistenceNotifierGuard<'a, fn() -> NotifyOption> {
28782880
PersistenceNotifierGuard {
28792881
event_persist_notifier: &cm.get_cm().event_persist_notifier,
28802882
needs_persist_flag: &cm.get_cm().needs_persist_flag,
2881-
should_persist: persist_check,
2883+
should_persist: Some(persist_check),
28822884
_read_guard: read_guard,
28832885
}
28842886
}
28852887
}
28862888

2887-
impl<'a, F: FnMut() -> NotifyOption> Drop for PersistenceNotifierGuard<'a, F> {
2889+
impl<'a, F: FnOnce() -> NotifyOption> Drop for PersistenceNotifierGuard<'a, F> {
28882890
fn drop(&mut self) {
2889-
match (self.should_persist)() {
2891+
let should_persist = match self.should_persist.take() {
2892+
Some(should_persist) => should_persist,
2893+
None => {
2894+
debug_assert!(false);
2895+
return;
2896+
},
2897+
};
2898+
match should_persist() {
28902899
NotifyOption::DoPersist => {
28912900
self.needs_persist_flag.store(true, Ordering::Release);
28922901
self.event_persist_notifier.notify()
@@ -7540,7 +7549,8 @@ where
75407549
ComplFunc: FnOnce(
75417550
Option<u64>,
75427551
bool,
7543-
) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
7552+
)
7553+
-> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
75447554
>(
75457555
&self, prev_hop: HTLCPreviousHopData, payment_preimage: PaymentPreimage,
75467556
payment_info: Option<PaymentClaimDetails>, completion_action: ComplFunc,

0 commit comments

Comments
 (0)