Matt Corallo [Sun, 12 Dec 2021 18:44:34 +0000 (18:44 +0000)]
Use the same FindClass argument for Android and regular Java
Its unclear whether the android variant (no L prefix, ; suffix) was
ever tested on normal Java or not, but the existing code generates
a large volumes of warnings on OpenJDK 11 like:
WARNING in native method: JNI FindClass received a bad class descriptor "Lorg/ldk/impl/bindings$LDKEvent$PaymentReceived;". A correct class descriptor has no leading "L" or trailing ";". Incorrect descriptors will not be accepted in future releases.
Matt Corallo [Sun, 12 Dec 2021 18:35:18 +0000 (18:35 +0000)]
Check exceptions after calling Enum.ordinal() from Rust
This generally shouldn't ever fire, but otherwise Java says, e.g.
WARNING in native method: JNI call made without checking exceptions when required to from CallIntMethod
at org.ldk.impl.bindings.create_invoice_from_channelmanager(Native Method)
Matt Corallo [Thu, 2 Dec 2021 18:24:05 +0000 (18:24 +0000)]
Clone objects being returned from trait methods before return from Java
When we return an object from a trait method called from Rust, we
often return complex Java "Human" objects. Because the underlying
object is owned by Java, we clone them before passing the objects
back to Rust, if possible. However, the clone call happens after
the Java method returns, at which point Java does not have any
references to the original "Human" object, which upon free will
free the underlying object.
While the time between when the Java method returns and the C FFI
code clones the object is incredibly short, CI did manage to find
the race here in ASAN, where the original object may be freed
before being accessed again for the clone in C.
Here we fix this by simply cloneing the object being returned
directly from Java.
Matt Corallo [Tue, 30 Nov 2021 01:38:04 +0000 (01:38 +0000)]
Check array lengths before passing them to C
When users pass a static-length array to C we currently CHECK its
length, asserting only if we are built in debug mode. In
production, we happily call JNI's `GetByteArrayRegion` with the
expected length, ignoring any errors. `GetByteArrayRegion`,
however, "THROWS ArrayIndexOutOfBoundsException: if one of the
indexes in the region is not valid.". While its somewhat unclear
what "THROWS" means in the context of a C API, it seems safe to
assume accessing return values after a "THROWS" condition is
undefined. Thus, we should ensure we check array lengths before
calling into C.
We do this here with a simple wrapper function added to
`org.ldk.util.InternalUtils` which checks an array is the correct
length before returning it.
Matt Corallo [Thu, 11 Nov 2021 17:34:41 +0000 (17:34 +0000)]
Hold a reference when we pass Option<Trait> to rust
This resolves a NPE when calling trait methods if the user doesn't
hold their own reference to the underlying trait, which is quite
possible for, eg, the `Filter` object.
We also adapt `HumanObjectPeerTest` to test this with a `Filter`.
Matt Corallo [Tue, 2 Nov 2021 17:51:11 +0000 (17:51 +0000)]
Add support for InvoicePayer to ChannelManagerConstructor and test
Note that this adds an additional bit of runs in the
HumanObjectPeerTest, bringing leaks for a full run to:
149 allocations remained for 1157302 bytes.
Matt Corallo [Tue, 19 Oct 2021 06:24:54 +0000 (06:24 +0000)]
Don't allocate a buffer to return a reference to a tuple element
If we're returning a reference to an object because we don't have a
clone function available, we'll set the reference flag implying no
free'ing will occur. In that case, we don't need to allocate a
buffer to copy the object's memory, we might as well just return a
pointer to the original.
After this commit, test leaks are:
73 allocations remained for 1137376 bytes.
Matt Corallo [Mon, 27 Sep 2021 23:31:55 +0000 (23:31 +0000)]
Correct and use ChannelManagerConstructor in HumanObjectPeerTest
When ChannelManagerConstructor support was added, the test loop
bounds were never updated to actually test it. This corrects that
(and a few regressions that snuck in over time).
Memory leaks during tests are now:
249 allocations remained for 1146062 bytes.
Matt Corallo [Mon, 27 Sep 2021 00:05:18 +0000 (00:05 +0000)]
Redo tuple mapping to be explicit and not generic
This was suggested by Galder as more Java-y, but also fixes
a number of memory leaks by avoiding the complexity of
always holding references and letting the common code do
more work.
Matt Corallo [Sat, 4 Sep 2021 20:39:32 +0000 (20:39 +0000)]
Rename ChannelManagerConstructor.ChannelManagerPersister to EventHandler
ChannelManagerPersister is a bit of a misnomer for the struct which
now handles events as well. Thus, we rename it to focus on the
event-handling function here.
Matt Corallo [Sat, 4 Sep 2021 19:46:46 +0000 (19:46 +0000)]
Drop dup PeerManager tiemr_tick_occurred calls from NioPeerHandler
We call PeerManager's timer_tick_occurred in the
lightning-background-processor crate, initialized from the
ChannelManagerConstructor. Prior to the use of the
lightning-background-processor we'd needed to call PeerManager's
timer_tick_occurred from NioPeerHandler, but we never dropped it
after the switch. Thus, every ~30 seconds we'll call
PeerManager::timer_tick_occurred twice in a row, disconnecting all
of our peers.