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);