On Mon, 15 Mar 2021 12:53:16 GMT, Jonathan Dowland <[email protected]> wrote:
>> This is an adaptation of a patch originally written by Shafi Ahmad in
>> a comment on the JBS page but never submitted or merged.
>>
>> With -Xcheck:jni, the method java.net.NetworkInterface.getAll very
>> quickly breaches the default JNI local refs threshold (32). Exactly when
>> this happens depends upon the number of network interfaces (in state "UP")
>> visible to Java. On Linux, with current Trunk, 2 network interfaces is
>> sufficient to breach:
>>
>> $ ./addif.sh 0
>> 1 interfaces
>> $ sudo ip netns exec jbase $JAVA_HOME/bin/java -Xcheck:jni NITest
>> (nothing)
>> $ ./addif.sh 1
>> 2 interfaces
>> $ sudo ip netns exec jbase $JAVA_HOME/bin/java -Xcheck:jni NITest
>> WARNING: JNI local refs: 33, exceeds capacity: 32
>> at java.net.NetworkInterface.getAll(java.base/Native Method)
>> at
>> java.net.NetworkInterface.getNetworkInterfaces(java.base/NetworkInterface.java:351)
>> at NITest.main(NITest.java:3)
>>
>> This patch improves the situation:
>>
>> $ ./addif.sh 3
>> 4 interfaces
>> $ sudo ip netns exec jbase $JAVA_HOME/bin/java -Xcheck:jni NITest
>> (nothing)
>> $ ./addif.sh 4
>> 5 interfaces
>> $ sudo ip netns exec jbase $JAVA_HOME/bin/java -Xcheck:jni NITest
>> WARNING: JNI local refs: 33, exceeds capacity: 32
>> at java.net.NetworkInterface.getAll(java.base/Native Method)
>> at
>> java.net.NetworkInterface.getNetworkInterfaces(java.base/NetworkInterface.java:351)
>> at NITest.main(NITest.java:3)
>>
>>
>> Once the JNI local refs threshold is breached, the threshold is raised.
>> With the patch, it takes 10 network interfaces to breach the new
>> threshold
>>
>> $ ./addif.sh 9
>> 10 interfaces
>> $ sudo ip netns exec jbase $JAVA_HOME/bin/java -Xcheck:jni NITest
>> WARNING: JNI local refs: 33, exceeds capacity: 32
>> at java.net.NetworkInterface.getAll(java.base/Native Method)
>> at
>> java.net.NetworkInterface.getNetworkInterfaces(java.base/NetworkInterface.java:351)
>> at NITest.main(NITest.java:3)
>> WARNING: JNI local refs: 66, exceeds capacity: 65
>> at java.net.NetworkInterface.getAll(java.base/Native Method)
>> at
>> java.net.NetworkInterface.getNetworkInterfaces(java.base/NetworkInterface.java:351)
>> at NITest.main(NITest.java:3)
>>
>> Without the patch it takes 5.
>>
>> Helper scripts for testing on Linux. `setupnet.sh`:
>>
>> #!/bin/bash
>> set -euo pipefail
>>
>> namespace=${namespace:-jbase}
>> sudo ip netns add ${namespace}
>>
>> And `addif.sh`:
>>
>> #!/bin/bash
>> set -euo pipefail
>>
>> namespace=${namespace:-jbase}
>> num="$1"
>>
>> sudo ip link add name vethhost${num} type veth peer name
>> veth${namespace}${num}
>> sudo ip link set veth${namespace}${num} netns ${namespace}
>> sudo ip addr add 192.168.2.${num}/24 dev vethhost${num}
>> sudo ip netns exec ${namespace} ip addr add 192.168.2.${num}/24 dev
>> veth${namespace}${num}
>> sudo ip link set vethhost${num} up
>> sudo ip netns exec ${namespace} ip link set veth${namespace}${num} up
>>
>> count="$(sudo ip netns exec ${namespace} ip link show |grep UP | wc -l)"
>> echo "${count} interfaces"
>
> Jonathan Dowland has refreshed the contents of this pull request, and
> previous commits have been removed. The incremental views will show
> differences compared to the previous content of the PR. The pull request
> contains one new commit since the last revision:
>
> 8187450: JNI local refs exceeds capacity warning in NetworkInterface::getAll
src/java.base/unix/native/libnet/NetworkInterface.c line 773:
> 771: return NULL;
> 772: }
> 773: (*env)->DeleteLocalRef(env, ia2Obj);
The pattern is 1) SetObjectXXX, followed by 2) DeleteLocalRef (once there is
another, non-local, reference). It might be worth relocating this line of code
to directly after the SetObjectField.
src/java.base/unix/native/libnet/NetworkInterface.c line 813:
> 811: addrP = addrP->next;
> 812: (*env)->DeleteLocalRef(env, iaObj);
> 813: (*env)->DeleteLocalRef(env, ibObj);
At the cost of duplicating the delete on ibObj, is it worth locating the
DeleteLocalRef to directly after the SetObjectArrayElement (so as to make it a
little easier to reason about whether the code has followed the set/delete
pattern correctly) ?
-------------
PR: https://git.openjdk.java.net/jdk/pull/2963
