Repository navigation
fix: keep audio device change handling from aborting or corrupting state - #337
Merged
Merged
Conversation
CoreAudioDeviceManager::deviceListenerProc and the IMMNotificationClient callbacks of WindowsAudioDeviceManager run on threads the operating system owns, and call code that throws jni::Exception when a device's properties cannot be read. A device that disappears between the device list and that read is an ordinary event, and the exception had no handler above it: std::terminate, and the JVM process aborted. The callbacks now log what was thrown and return normally. Fixes #332
…resh The device change callbacks now catch what a refresh throws (#333), but on macOS a throw still cut the refresh short. One device that went away between reading the device list and reading its name ended onDevicesChanged() there: the devices after it were not added, and neither list was checked for removed devices, so a disconnected device stayed listed with no event for it. On a Java thread the same throw failed MediaDevices.getAudioCaptureDevices() and getAudioRenderDevices() as a whole. createAudioDevice() now logs such a device and returns nullptr, which both enumerations already skip. createDefaultAudioDevice() returns nullptr when there is no default device instead of reading device 0, and the default device getters return null to Java rather than dereferencing it. checkDeviceGone() looked up the default device by reading it again from CoreAudio, which fails when the default is the device that has just gone, and threw after removing the device but before reporting it. The device it read was a new object and so never equal to the removed one, which made the update it guarded dead code. It now compares with the default the manager holds, and clears it and takes the system's new default from the list when the removed device was the default. The device IDs are held in a vector, which a throw no longer leaks, and only as many are read as CoreAudio returned on the second call. On Windows, fillAdditionalTypes() threw when an endpoint could not be read, which failed the whole enumeration for the transport and form factor of one device. Such a device now keeps the unknown ones. The default device getters threw when there is no device of the kind, as on a machine without a microphone; they now return null there too, as they do on macOS.
DeviceChangeListener calls into Java on a thread the operating system owns, and did not look for an exception afterwards. One thrown by a listener stayed pending on that thread, so the next listener and the next device event made their JNI calls with an exception pending, which JNI does not allow and which -Xcheck:jni reports as fatal. The exception is now described and cleared. Converting the device to Java can throw a C++ exception too, and on the PulseAudio, udev and AVFoundation threads nothing above the listener catches it. The listener now logs it and returns.
MediaDevices.addDeviceChangeListener() and removeDeviceChangeListener() change a device manager's listener list on a Java thread, while the operating system reports device changes on its own thread, which walks the list and erases expired entries from it. Nothing synchronized the two. A listener could also expire between the check for it and its lock(), which then returned null. The list is now guarded by a mutex. The listeners are called on a copy taken under it, outside the lock, so that one may remove itself from within its callback.
…registry JavaContext::removeNativeRef() made a JavaGlobalRef to compare with, deleted it with DeleteGlobalRef() and then let its destructor delete it again. Every removal of a track or device change listener freed a global reference twice: -Xcheck:jni aborts on it, and with several threads the second delete can free a reference another thread has just been given. The listener is now compared with the reference the caller passed, so there is nothing to delete. The registry of listeners was not synchronized either, while listeners are added and removed on any Java thread. It now has a mutex; the class name of the listener is looked up before it is taken. AudioTrackTests adds and removes track listeners, alone and from eight threads at once. Before the fix, under -Pjni-check, they aborted the JVM with "Bad global or local ref passed to JNI".
… alone The native registry of Java listeners found an entry by the listener object only, and took the first match. A listener added to two tracks and removed from the second had the first track's observer removed instead: it was freed while the first track still called it, and the second track went on delivering events to a listener that had been removed. The same happened with one object registered as both a mute and an ended listener, where removing it as one kind took the other. An entry now also carries its owner, the native track, and its kind, and is only found by all three. Device change listeners keep the defaults, as they have no owner. A disposed track now takes its remaining listeners off before it is released. Their entries were left behind with the track's address, which a later track can be given, and the Java listeners they hold were never released. AudioTrackTests covers a listener on two tracks, a listener of both kinds, and disposing a track with listeners. Before the fix, running them crashed the JVM, and the test of a listener of both kinds failed on its own: no mute event arrived.
# Conflicts: # webrtc-jni/src/main/cpp/src/media/audio/macos/CoreAudioDeviceManager.cpp
SendableMetatype
added a commit
to EduGeyser/webrtc-java
that referenced
this pull request
Oct 10, 2026
Brings in the work upstream added on October 10: the missing peer connection signaling and control calls (devopvoid#334), callback exceptions kept from unwinding through WebRTC (devopvoid#335), first-packet observers, sender and receiver IDs and track volume (devopvoid#338), video processors for camera and desktop frames (devopvoid#342), and fixes to audio device changes, camera hotplug and media stream views (devopvoid#333, devopvoid#337, devopvoid#341, devopvoid#343). Fork-side adaptations: - setAudioPlayout and setAudioRecording reach the voice engine, so the data channels variant leaves them out, like the other audio natives. - The new tests that need audio or video carry the media tag. - The Maven publish action stays deleted.
SendableMetatype
added a commit
to EduGeyser/webrtc-java
that referenced
this pull request
Oct 10, 2026
Brings in the work upstream added on October 10: the missing peer connection signaling and control calls (devopvoid#334), callback exceptions kept from unwinding through WebRTC (devopvoid#335), first-packet observers, sender and receiver IDs and track volume (devopvoid#338), video processors for camera and desktop frames (devopvoid#342), and fixes to audio device changes, camera hotplug and media stream views (devopvoid#333, devopvoid#337, devopvoid#341, devopvoid#343). Fork-side adaptations: - setBitrate, setAudioPlayout and setAudioRecording reach the call and the voice engine, which a factory without a media engine does not create, so the data channels variant leaves them out, like the other media natives. - The new tests that need audio or video carry the media tag. - The Maven publish action stays deleted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups to #333 (by @Yml-0, already merged as 1bbef2a), which made the macOS CoreAudio listener and the Windows
IMMNotificationClientcallbacks catch what a device refresh throws instead of letting it end the process. This branch was built on #333's original commit and hasmainmerged back in; its diff againstmainis the follow-ups alone, so any merge method works, a squash included.Follow-ups
onDevicesChanged()short, so later new devices were not added and removed devices stayed listed with no event.createAudioDevice()now skips such a device.checkDeviceGone()compared the removed device with a newly read default (never equal, and reading a vanished device throws); it now uses the default the manager holds. The device-ID buffer is astd::vector. On Windows, a device whose transport or form factor cannot be read keeps the unknown ones rather than failing the enumeration. The default-device getters returnnullwhen there is no such device, on both platforms; before, macOS would have dereferenced a null device and Windows threw.DeviceChangeListenernever throws, and clears what a Java listener throws. It used to leave the Java exception pending on the OS-owned thread, for the next listener or the next event to make JNI calls with.JavaContext::removeNativeReffreed a global reference twice on every listener removal (track listeners and device change listeners), and its registry had no lock. Found while testing the item above;-Xcheck:jniaborts on it.Testing
Windows x64:
mvn -pl webrtc-jni,webrtc verifybuilds, and thewebrtcsuite passes (237 tests, 0 failures, 2 skipped), also under-Pjni-checkwith no fatal JNI errors. New tests inAudioTrackTestsadd and remove track listeners, alone and from eight threads; before theJavaContextfix they aborted the JVM under-Pjni-check. Further tests cover one listener on two tracks, one object as both kinds of listener, and disposing a track with listeners; before the keying fix they crashed the JVM, and the test of both kinds failed on its own (no mute event arrived). A local check, not committed, enumerated devices, read the defaults (nullfor capture on a machine without a microphone) and added and removed device change listeners from eight threads under-Xcheck:jni.The macOS changes have not been compiled locally; this PR's CI is their first build. Device hotplug cannot be triggered from a test, so the callback paths are covered by review only.