-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[Android]Fix Jni local and global reference leaks for generic IM #29236
[Android]Fix Jni local and global reference leaks for generic IM #29236
Conversation
PR #29236: Size comparison from f47ce53 to d90f406 Full report (61 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
d90f406
to
b1b4d86
Compare
PR #29236: Size comparison from f47ce53 to b1b4d86 Decreases (1 build for efr32)
Full report (38 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
b1b4d86
to
a391563
Compare
a391563
to
35c1a0b
Compare
PR #29236: Size comparison from 73c327e to 34e954c Full report (4 builds for cc32xx, mbed, qpg)
|
1b53d1d
to
021da74
Compare
PR #29236: Size comparison from 73c327e to 021da74 Full report (54 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
PR #29236: Size comparison from 73c327e to 8fbaa36 Full report (43 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
PR #29236: Size comparison from 73c327e to d21756f Full report (4 builds for cc32xx, mbed, qpg)
|
97dd236
to
c8df5ad
Compare
PR #29236: Size comparison from 24288ec to c8df5ad Full report (27 builds for cc13x4_26x4, cc32xx, cyw30739, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
PR #29236: Size comparison from 24288ec to 9377ae4 Increases (1 build for bl702l)
Decreases (1 build for esp32)
Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
9377ae4
to
6e31532
Compare
PR #29236: Size comparison from f03e9ef to 6e31532 Full report (39 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
6e31532
to
fd37f57
Compare
PR #29236: Size comparison from f03e9ef to fd37f57 Increases (1 build for telink)
Decreases (1 build for telink)
Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
During wildcard read/subscription test, we see 6k+ local reference leaks, which further block java garbage collection to release the corresponding objects from IM in java/kotlin side.
From jni spec, "All local references created during the execution of a native method will be freed once the native method returns." it only applies when a native Java method is called from the VM, otherwise, we should delete the local references explicitly and manually. In addition, for any long-lived jni reference, we should use global reference per spec.
In our code, we see these local references have not yet been deleted.
Release all local referenced for IM via introducing JniLocalReferenceManager which use JNI recommended PushLocalFrame and PoplocalFrame to bulk delete local references.
For the RAII Jni class, like JNIChipAttribute, we use JNII to release global refence,
For others where the function return the local reference object, like JniReferences::GetClassRef, we wrap it with globalReference, then release via RAII JniObject to release global reference as well.
For Java NodeState constructor, we create hashmap inside instead of creating hashmap via JNI(not recommended).
Fix several global reference leak in controllerCallback.
Test: Local reproduce and verify there is no complain on reference leaks during commissioning + IM interactions in both java matter controller and android chip-tool
#29069