From 95e531b49843505eb7ce56d8ecc6ffbdf566a4c9 Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Wed, 16 Sep 2026 23:36:26 +0200 Subject: [PATCH] fix: take Map method IDs from the interface instead of from HashMap jni::JavaHashMap resolved put() and entrySet() against java.util.HashMap and then called them on whatever map it held. One of its constructors takes any java.util.Map, and PeerConnectionFactory hands it the field trials map the application passed in, which is rarely a HashMap. Collections.singletonMap, Collections.emptyMap, Map.of and TreeMap are all other classes. Using a method ID from one class on an object of another is undefined. It happens to work on HotSpot, so the tests pass, but the JVM aborts under -Xcheck:jni: FATAL ERROR in native method: Wrong object class or methodID passed to JNI call at dev.onvoid.webrtc.PeerConnectionFactory.initialize(Native Method) It aborted partway through the suite, which left "mvn -pl webrtc test -Pjni-check" unusable as a gate on any branch. Both methods now come from the java.util.Map interface, where virtual dispatch handles whichever implementation arrives, the way JavaMapIterator already resolves Set, Iterator and Map.Entry. The class keeps its HashMap reference for the constructor that creates one. RTCRtpCodecCapability, RTCRtpCodecParameters and RTCStats wrap caller-supplied maps through the same helper and are fixed along with it. The new test builds the factory from several map implementations, none of them a HashMap. It passes either way on its own and only fails the -Pjni-check profile, which is where this class of bug is visible. --- .../jni-voithos/src/JavaHashMap.cpp | 14 ++++++-- .../webrtc/PeerConnectionFactoryTests.java | 36 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaHashMap.cpp b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaHashMap.cpp index 64cc94eb..122dfeba 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaHashMap.cpp +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaHashMap.cpp @@ -54,7 +54,17 @@ namespace jni cls = FindClass(env, "java/util/HashMap"); defaultCtor = GetMethod(env, cls, "", "()V"); - put = GetMethod(env, cls, "put", "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;"); - entrySet = GetMethod(env, cls, "entrySet", "()Ljava/util/Set;"); + + // put() and entrySet() are called on the map this class holds, which is + // a HashMap it created itself only when constructed without one. The + // other constructor takes any java.util.Map, and a method ID taken from + // HashMap may not be used on a map of another class: the JVM aborts with + // "Wrong object class or methodID passed to JNI call" under -Xcheck:jni, + // and is free to misbehave without it. Taking these from the interface + // leaves the call to virtual dispatch and works for either map. + jclass mapClass = FindClass(env, "java/util/Map"); + + put = GetMethod(env, mapClass, "put", "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;"); + entrySet = GetMethod(env, mapClass, "entrySet", "()Ljava/util/Set;"); } } diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/PeerConnectionFactoryTests.java b/webrtc/src/test/java/dev/onvoid/webrtc/PeerConnectionFactoryTests.java index 467c89ca..f11b3df4 100644 --- a/webrtc/src/test/java/dev/onvoid/webrtc/PeerConnectionFactoryTests.java +++ b/webrtc/src/test/java/dev/onvoid/webrtc/PeerConnectionFactoryTests.java @@ -25,8 +25,12 @@ import dev.onvoid.webrtc.media.video.VideoDeviceSource; import dev.onvoid.webrtc.media.video.VideoTrack; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.TreeMap; +import java.util.concurrent.ConcurrentHashMap; import org.junit.jupiter.api.Test; @@ -80,6 +84,38 @@ void createWithEmptyFieldTrials() { factory.dispose(); } + @Test + void createWithFieldTrialsOfAnyMapType() { + // The field trials are read natively through a helper that used to take + // its method IDs from java.util.HashMap and call them on whatever map it + // was given. Anything but a HashMap was then undefined behaviour, which + // happened to work but aborts the JVM under -Xcheck:jni. Every map here + // is a different implementation, and none of them is a HashMap. + // LinkedHashMap is deliberately absent: it extends HashMap, so it would + // pass either way. + Map treeMap = new TreeMap<>(); + treeMap.put("WebRTC-Bar", "Enabled"); + treeMap.put("WebRTC-Foo", "Disabled"); + + Map concurrentMap = new ConcurrentHashMap<>(); + concurrentMap.put("WebRTC-Bar", "Enabled"); + + List> fieldTrialMaps = Arrays.asList( + Collections.singletonMap("WebRTC-Bar", "Enabled"), + Collections.emptyMap(), + treeMap, + concurrentMap, + Collections.unmodifiableMap(treeMap)); + + for (Map fieldTrials : fieldTrialMaps) { + AudioDeviceModule audioDevModule = new AudioDeviceModule(AudioLayer.kDummyAudio); + PeerConnectionFactory factory = new PeerConnectionFactory(fieldTrials, audioDevModule); + + factory.dispose(); + audioDevModule.dispose(); + } + } + @Test void createWithInvalidFieldTrials() { AudioDeviceModule audioDevModule = new AudioDeviceModule(AudioLayer.kDummyAudio);