From b479a721c55c876e716319c5074c7d150a59aa5c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 8 Mar 2023 05:04:14 +0000 Subject: [PATCH] [Java] Expose the ProbabilisticScorer from the CMC correctly We'd previously exposed a race-y version of the ProbabilisticScorer where its held within a lock but also made public with no threading requirements placed on it. This resolves that issue by exposing it via a wrapper that holds the score lock. --- .../batteries/ChannelManagerConstructor.java | 24 ++++++++++++++++++- .../java/org/ldk/HumanObjectPeerTest.java | 20 ++++++++++++---- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/ldk/batteries/ChannelManagerConstructor.java b/src/main/java/org/ldk/batteries/ChannelManagerConstructor.java index 029fac79..db37118d 100644 --- a/src/main/java/org/ldk/batteries/ChannelManagerConstructor.java +++ b/src/main/java/org/ldk/batteries/ChannelManagerConstructor.java @@ -76,10 +76,32 @@ public class ChannelManagerConstructor { * we want to expose underlying details of the scorer itself. Thus, we expose a safe version that takes the lock * then returns a reference to this scorer. */ - @Nullable private final ProbabilisticScorer prob_scorer; + private final ProbabilisticScorer prob_scorer; private final Logger logger; private final KeysManager keys_manager; + /** + * Exposes the `ProbabilisticScorer` wrapped inside a lock. Don't forget to `close` this lock when you're done with + * it so normal scoring operation can continue. + */ + public class ScorerWrapper implements AutoCloseable { + private final Score lock; + public final ProbabilisticScorer prob_scorer; + private ScorerWrapper(Score lock, ProbabilisticScorer prob_scorer) { + this.lock = lock; this.prob_scorer = prob_scorer; + } + @Override public void close() throws Exception { + lock.destroy(); + } + } + /** + * Gets the `ProbabilisticScorer` which backs the public lockable `scorer`. Don't forget to `close` the lock when + * you're done with it. + */ + public ScorerWrapper get_locked_scorer() { + return new ScorerWrapper(this.scorer.as_LockableScore().lock(), this.prob_scorer); + } + /** * Deserializes a channel manager and a set of channel monitors from the given serialized copies and interface implementations * diff --git a/src/test/java/org/ldk/HumanObjectPeerTest.java b/src/test/java/org/ldk/HumanObjectPeerTest.java index 113eae90..0fa0c35f 100644 --- a/src/test/java/org/ldk/HumanObjectPeerTest.java +++ b/src/test/java/org/ldk/HumanObjectPeerTest.java @@ -946,7 +946,7 @@ class HumanObjectPeerTestInstance { this.best_blockhash = best_blockhash; } } - void do_test_message_handler_b(TestState state) throws InterruptedException { + void do_test_message_handler_b(TestState state) throws Exception { GcCheck obj = new GcCheck(); if (state.ref_block != null) { // Ensure the original peers get freed before we move on. Note that we have to be in a different function @@ -1128,6 +1128,16 @@ class HumanObjectPeerTestInstance { assert upd_msg instanceof Result_ChannelUpdateDecodeErrorZ.Result_ChannelUpdateDecodeErrorZ_OK; assert ((Result_ChannelUpdateDecodeErrorZ.Result_ChannelUpdateDecodeErrorZ_OK) upd_msg).res.get_contents().get_htlc_maximum_msat() == 0xdeadbeef42424242L; Option_NetworkUpdateZ upd = Option_NetworkUpdateZ.some(NetworkUpdate.channel_update_message(((Result_ChannelUpdateDecodeErrorZ.Result_ChannelUpdateDecodeErrorZ_OK) upd_msg).res)); + + if (use_chan_manager_constructor) { + // Lock the scorer twice back-to-back to check that the try-with-resources AutoCloseable on the scorer works. + try (ChannelManagerConstructor.ScorerWrapper score = state.peer1.constructor.get_locked_scorer()) { + score.prob_scorer.debug_log_liquidity_stats(); + } + try (ChannelManagerConstructor.ScorerWrapper score = state.peer1.constructor.get_locked_scorer()) { + score.prob_scorer.clear_manual_penalties(); + } + } } java.util.LinkedList> must_free_objs = new java.util.LinkedList(); @@ -1143,13 +1153,13 @@ class HumanObjectPeerTestInstance { } } public class HumanObjectPeerTest { - static HumanObjectPeerTestInstance do_test_run(boolean nice_close, boolean use_km_wrapper, boolean use_manual_watch, boolean reload_peers, boolean break_cross_peer_refs, boolean nio_peer_handler, boolean use_ignoring_routing_handler, boolean use_chan_manager_constructor, boolean use_invoice_payer) throws InterruptedException { + static HumanObjectPeerTestInstance do_test_run(boolean nice_close, boolean use_km_wrapper, boolean use_manual_watch, boolean reload_peers, boolean break_cross_peer_refs, boolean nio_peer_handler, boolean use_ignoring_routing_handler, boolean use_chan_manager_constructor, boolean use_invoice_payer) throws Exception { HumanObjectPeerTestInstance instance = new HumanObjectPeerTestInstance(nice_close, use_km_wrapper, use_manual_watch, reload_peers, break_cross_peer_refs, nio_peer_handler, !nio_peer_handler, use_ignoring_routing_handler, use_chan_manager_constructor, use_invoice_payer); HumanObjectPeerTestInstance.TestState state = instance.do_test_message_handler(); instance.do_test_message_handler_b(state); return instance; } - static void do_test(boolean nice_close, boolean use_km_wrapper, boolean use_manual_watch, boolean reload_peers, boolean break_cross_peer_refs, boolean nio_peer_handler, boolean use_ignoring_routing_handler, boolean use_chan_manager_constructor, boolean use_invoice_payer) throws InterruptedException { + static void do_test(boolean nice_close, boolean use_km_wrapper, boolean use_manual_watch, boolean reload_peers, boolean break_cross_peer_refs, boolean nio_peer_handler, boolean use_ignoring_routing_handler, boolean use_chan_manager_constructor, boolean use_invoice_payer) throws Exception { HumanObjectPeerTestInstance instance = do_test_run(nice_close, use_km_wrapper, use_manual_watch, reload_peers, break_cross_peer_refs, nio_peer_handler, use_ignoring_routing_handler, use_chan_manager_constructor, use_invoice_payer); while (instance.gc_count != instance.gc_exp_count) { System.gc(); @@ -1159,7 +1169,7 @@ public class HumanObjectPeerTest { assert o.get() == null; } public static final int TEST_ITERATIONS = (1 << 9); - public static void do_test_message_handler(IntConsumer statusFunction) throws InterruptedException { + public static void do_test_message_handler(IntConsumer statusFunction) throws Exception { Thread gc_thread = new Thread(() -> { while (true) { System.gc(); @@ -1209,7 +1219,7 @@ public class HumanObjectPeerTest { gc_thread.join(); } @Test - public void test_message_handler() throws InterruptedException { + public void test_message_handler() throws Exception { do_test_message_handler(i -> System.err.println("Running test with flags " + i)); } -- 2.39.5