Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0.0.103.1 #56

Merged
merged 18 commits into from
Dec 3, 2021
Merged

v0.0.103.1 #56

merged 18 commits into from
Dec 3, 2021

Conversation

TheBlueMatt
Copy link
Collaborator

@TheBlueMatt TheBlueMatt commented Nov 11, 2021

Fixes #55, #57, #59, #60, #62.

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`.
@TheBlueMatt TheBlueMatt changed the title Next Version v0.0.103.1 Nov 30, 2021
@TheBlueMatt TheBlueMatt force-pushed the main branch 5 times, most recently from 91501f2 to 4ccc9c8 Compare December 1, 2021 06:59
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.
@TheBlueMatt TheBlueMatt force-pushed the main branch 4 times, most recently from e779c9c to 59ec115 Compare December 1, 2021 21:47
@TheBlueMatt TheBlueMatt force-pushed the main branch 4 times, most recently from 1fb69fb to 1f99c9c Compare December 2, 2021 19:28
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.
As of this commit, leaks for a full run are down to:
    17 allocations remained for 1115062 bytes.
@TheBlueMatt TheBlueMatt force-pushed the main branch 2 times, most recently from ddf10d2 to ccfa10a Compare December 2, 2021 20:32
@TheBlueMatt TheBlueMatt merged commit 6d094e7 into lightningdevkit:main Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't provide custom UserConfig in both ChannelManagerConstructor constructors
1 participant