From 77557668dfe717d214f53f2f089cb80f653584f8 Mon Sep 17 00:00:00 2001 From: Artem Suvorov Date: Thu, 8 Oct 2026 15:04:32 +0300 Subject: [PATCH 1/6] fix: keep exceptions inside the audio device change callbacks 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 --- .../audio/macos/CoreAudioDeviceManager.cpp | 48 ++++++++++------- .../windows/WindowsAudioDeviceManager.cpp | 51 ++++++++++++++----- 2 files changed, 70 insertions(+), 29 deletions(-) diff --git a/webrtc-jni/src/main/cpp/src/media/audio/macos/CoreAudioDeviceManager.cpp b/webrtc-jni/src/main/cpp/src/media/audio/macos/CoreAudioDeviceManager.cpp index cb6e12ec..97f03c7f 100644 --- a/webrtc-jni/src/main/cpp/src/media/audio/macos/CoreAudioDeviceManager.cpp +++ b/webrtc-jni/src/main/cpp/src/media/audio/macos/CoreAudioDeviceManager.cpp @@ -22,6 +22,8 @@ #include #include +#include + #if !defined(MAC_OS_VERSION_12_0) || MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0 #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster #endif @@ -388,28 +390,40 @@ namespace jni OSStatus CoreAudioDeviceManager::deviceListenerProc(AudioObjectID objectID, UInt32 numberAddresses, const AudioObjectPropertyAddress addresses[], void * clientData) { CoreAudioDeviceManager * manager = static_cast(clientData); + // This runs on a CoreAudio HAL queue. An exception that leaves it has no handler above + // it and terminates the process, so nothing thrown below may escape: a device that + // vanishes between the device list and the read of its properties is an ordinary event + // (Bluetooth profile switch, an aggregate device torn down), not a fatal one. for (int i = 0; i < numberAddresses; i++) { - switch (addresses[i].mSelector) { - case kAudioHardwarePropertyDevices: - { - manager->onDevicesChanged(); - break; - } + try { + switch (addresses[i].mSelector) { + case kAudioHardwarePropertyDevices: + { + manager->onDevicesChanged(); + break; + } - case kAudioHardwarePropertyDefaultInputDevice: - { - AudioDevicePtr def = manager->getDefaultCaptureDevice(); - manager->onDefaultDeviceChanged(kAudioObjectPropertyScopeInput, manager->captureDevices, def); - break; - } + case kAudioHardwarePropertyDefaultInputDevice: + { + AudioDevicePtr def = manager->getDefaultCaptureDevice(); + manager->onDefaultDeviceChanged(kAudioObjectPropertyScopeInput, manager->captureDevices, def); + break; + } - case kAudioHardwarePropertyDefaultOutputDevice: - { - AudioDevicePtr defp = manager->getDefaultPlaybackDevice(); - manager->onDefaultDeviceChanged(kAudioObjectPropertyScopeOutput, manager->playbackDevices, defp); - break; + case kAudioHardwarePropertyDefaultOutputDevice: + { + AudioDevicePtr defp = manager->getDefaultPlaybackDevice(); + manager->onDefaultDeviceChanged(kAudioObjectPropertyScopeOutput, manager->playbackDevices, defp); + break; + } } } + catch (const std::exception & e) { + RTC_LOG(LS_ERROR) << "CoreAudio: Device change handling failed: " << e.what(); + } + catch (...) { + RTC_LOG(LS_ERROR) << "CoreAudio: Device change handling failed"; + } } return noErr; diff --git a/webrtc-jni/src/main/cpp/src/media/audio/windows/WindowsAudioDeviceManager.cpp b/webrtc-jni/src/main/cpp/src/media/audio/windows/WindowsAudioDeviceManager.cpp index 705d8dd7..2a46bf1b 100644 --- a/webrtc-jni/src/main/cpp/src/media/audio/windows/WindowsAudioDeviceManager.cpp +++ b/webrtc-jni/src/main/cpp/src/media/audio/windows/WindowsAudioDeviceManager.cpp @@ -27,6 +27,7 @@ #include "modules/audio_device/include/audio_device.h" #include "rtc_base/logging.h" +#include #include namespace jni @@ -408,7 +409,17 @@ namespace jni { RTC_LOG(LS_INFO) << "MMF: Device added: " << deviceId; - addDevice(deviceId); + // IMMNotificationClient callbacks run on a system thread. An exception that leaves one + // has no handler above it and terminates the process, so it is logged and dropped. + try { + addDevice(deviceId); + } + catch (const std::exception & e) { + RTC_LOG(LS_ERROR) << "MMF: Device added handling failed: " << e.what(); + } + catch (...) { + RTC_LOG(LS_ERROR) << "MMF: Device added handling failed"; + } return S_OK; } @@ -417,7 +428,15 @@ namespace jni { RTC_LOG(LS_INFO) << "MMF: Device removed: " << deviceId; - removeDevice(deviceId); + try { + removeDevice(deviceId); + } + catch (const std::exception & e) { + RTC_LOG(LS_ERROR) << "MMF: Device removed handling failed: " << e.what(); + } + catch (...) { + RTC_LOG(LS_ERROR) << "MMF: Device removed handling failed"; + } return S_OK; } @@ -430,16 +449,24 @@ namespace jni return S_OK; } - switch (newState) { - case DEVICE_STATE_ACTIVE: - addDevice(deviceId); - break; - - case DEVICE_STATE_DISABLED: - case DEVICE_STATE_NOTPRESENT: - case DEVICE_STATE_UNPLUGGED: - removeDevice(deviceId); - break; + try { + switch (newState) { + case DEVICE_STATE_ACTIVE: + addDevice(deviceId); + break; + + case DEVICE_STATE_DISABLED: + case DEVICE_STATE_NOTPRESENT: + case DEVICE_STATE_UNPLUGGED: + removeDevice(deviceId); + break; + } + } + catch (const std::exception & e) { + RTC_LOG(LS_ERROR) << "MMF: Device state change handling failed: " << e.what(); + } + catch (...) { + RTC_LOG(LS_ERROR) << "MMF: Device state change handling failed"; } return S_OK; From 57af681b068706f63beaed84d8acffab1ca75901 Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Sat, 10 Oct 2026 00:08:10 +0200 Subject: [PATCH 2/6] fix: skip an audio device that cannot be read rather than end the refresh 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. --- .../src/main/cpp/src/JNI_MediaDevices.cpp | 8 +++ .../audio/macos/CoreAudioDeviceManager.cpp | 54 +++++++++++-------- .../windows/WindowsAudioDeviceManager.cpp | 32 +++++++---- 3 files changed, 63 insertions(+), 31 deletions(-) diff --git a/webrtc-jni/src/main/cpp/src/JNI_MediaDevices.cpp b/webrtc-jni/src/main/cpp/src/JNI_MediaDevices.cpp index 125ec53b..f78aa1cb 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_MediaDevices.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_MediaDevices.cpp @@ -68,6 +68,10 @@ JNIEXPORT jobject JNICALL Java_dev_onvoid_webrtc_media_MediaDevices_getDefaultAu try { auto device = context->getAudioDeviceManager()->getDefaultAudioPlaybackDevice(); + if (device == nullptr) { + return nullptr; + } + return jni::AudioDevice::toJavaAudioDevice(env, device).release(); } catch (...) { @@ -85,6 +89,10 @@ JNIEXPORT jobject JNICALL Java_dev_onvoid_webrtc_media_MediaDevices_getDefaultAu try { auto device = context->getAudioDeviceManager()->getDefaultAudioCaptureDevice(); + if (device == nullptr) { + return nullptr; + } + return jni::AudioDevice::toJavaAudioDevice(env, device).release(); } catch (...) { diff --git a/webrtc-jni/src/main/cpp/src/media/audio/macos/CoreAudioDeviceManager.cpp b/webrtc-jni/src/main/cpp/src/media/audio/macos/CoreAudioDeviceManager.cpp index 97f03c7f..66ebbca1 100644 --- a/webrtc-jni/src/main/cpp/src/media/audio/macos/CoreAudioDeviceManager.cpp +++ b/webrtc-jni/src/main/cpp/src/media/audio/macos/CoreAudioDeviceManager.cpp @@ -23,6 +23,7 @@ #include #include +#include #if !defined(MAC_OS_VERSION_12_0) || MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0 #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster @@ -94,12 +95,14 @@ namespace jni OSStatus status = AudioObjectGetPropertyDataSize(kAudioObjectSystemObject, &pa, 0, nullptr, &dataSize); ThrowIfFailed(status, "CoreAudio: Enumerate audio endpoints failed"); - const int numDevices = dataSize / sizeof(AudioDeviceID); - AudioDeviceID * devIDs = new AudioDeviceID[numDevices]; + std::vector devIDs(dataSize / sizeof(AudioDeviceID)); - status = AudioObjectGetPropertyData(kAudioObjectSystemObject, &pa, 0, nullptr, &dataSize, devIDs); + status = AudioObjectGetPropertyData(kAudioObjectSystemObject, &pa, 0, nullptr, &dataSize, devIDs.data()); ThrowIfFailed(status, "CoreAudio: Enumerate audio endpoints failed"); + // The list may have shrunk between the two calls. + const int numDevices = dataSize / sizeof(AudioDeviceID); + AudioDeviceID defaultID = getDefaultDeviceID(scope); for (int i = 0; i < numDevices; i++) { @@ -119,8 +122,6 @@ namespace jni } } } - - delete[] devIDs; } void CoreAudioDeviceManager::onDevicesChanged() @@ -136,12 +137,14 @@ namespace jni OSStatus status = AudioObjectGetPropertyDataSize(kAudioObjectSystemObject, &pa, 0, nullptr, &dataSize); ThrowIfFailed(status, "CoreAudio: Enumerate audio endpoints failed"); - const int numDevices = dataSize / sizeof(AudioDeviceID); - AudioDeviceID * devIDs = new AudioDeviceID[numDevices]; + std::vector devIDs(dataSize / sizeof(AudioDeviceID)); - status = AudioObjectGetPropertyData(kAudioObjectSystemObject, &pa, 0, nullptr, &dataSize, devIDs); + status = AudioObjectGetPropertyData(kAudioObjectSystemObject, &pa, 0, nullptr, &dataSize, devIDs.data()); ThrowIfFailed(status, "CoreAudio: Enumerate audio endpoints failed"); + // The list may have shrunk between the two calls. + const int numDevices = dataSize / sizeof(AudioDeviceID); + // Check, if a new device is available. for (int i = 0; i < numDevices; i++) { bool found = false; @@ -201,10 +204,8 @@ namespace jni } // Check, if a device was disconnected. - checkDeviceGone(captureDevices, devIDs, numDevices, kAudioObjectPropertyScopeInput); - checkDeviceGone(playbackDevices, devIDs, numDevices, kAudioObjectPropertyScopeOutput); - - delete[] devIDs; + checkDeviceGone(captureDevices, devIDs.data(), numDevices, kAudioObjectPropertyScopeInput); + checkDeviceGone(playbackDevices, devIDs.data(), numDevices, kAudioObjectPropertyScopeOutput); } void CoreAudioDeviceManager::onDefaultDeviceChanged(const AudioObjectPropertyScope & scope, DeviceList & devices, const AudioDevicePtr & device) @@ -256,19 +257,20 @@ namespace jni AudioDevicePtr removed = devices.removeDevice(predicate); if (removed != nullptr) { + // Compare with the default this manager holds, not with a newly read one: a new + // device object never equals the removed one, and reading a device that has just + // gone fails. The system's new default, if any, is looked up in the list. if (scope == kAudioObjectPropertyScopeInput) { - AudioDevicePtr def = getDefaultAudioCaptureDevice(); - - if (removed == def) { - onDefaultDeviceChanged(scope, captureDevices, def); + if (removed == getDefaultCaptureDevice()) { + setDefaultCaptureDevice(nullptr); + onDefaultDeviceChanged(scope, captureDevices, nullptr); } removed->directionType = AudioDeviceDirectionType::adtCapture; } else if (scope == kAudioObjectPropertyScopeOutput) { - AudioDevicePtr def = getDefaultAudioPlaybackDevice(); - - if (removed == def) { - onDefaultDeviceChanged(scope, playbackDevices, def); + if (removed == getDefaultPlaybackDevice()) { + setDefaultPlaybackDevice(nullptr); + onDefaultDeviceChanged(scope, playbackDevices, nullptr); } removed->directionType = AudioDeviceDirectionType::adtRender; } @@ -281,6 +283,10 @@ namespace jni { AudioDeviceID defaultID = getDefaultDeviceID(scope); + if (defaultID == kAudioObjectUnknown) { + return nullptr; + } + return createAudioDevice(defaultID, scope); } @@ -295,7 +301,13 @@ namespace jni pa.mElement = kAudioObjectPropertyElementMain; OSStatus status = AudioObjectGetPropertyData(deviceID, &pa, 0, nullptr, &dataSize, &devNameRef); - ThrowIfFailed(status, "CoreAudio: Get device name failed"); + + // A device can go away between reading the device list and reading its properties. + // It is skipped, so that one such device does not cut short the whole enumeration. + if (status != noErr || devNameRef == nullptr) { + RTC_LOG(LS_WARNING) << "CoreAudio: Get device name failed: Device = " << deviceID << ", Status = " << status; + return nullptr; + } std::string name = CFStringRefToUTF8(devNameRef); std::string id = std::to_string(deviceID); diff --git a/webrtc-jni/src/main/cpp/src/media/audio/windows/WindowsAudioDeviceManager.cpp b/webrtc-jni/src/main/cpp/src/media/audio/windows/WindowsAudioDeviceManager.cpp index 2a46bf1b..216c08ed 100644 --- a/webrtc-jni/src/main/cpp/src/media/audio/windows/WindowsAudioDeviceManager.cpp +++ b/webrtc-jni/src/main/cpp/src/media/audio/windows/WindowsAudioDeviceManager.cpp @@ -168,16 +168,22 @@ namespace jni hr = deviceEnumerator->GetDevice(wDeviceID, &pDevice); delete[] wDeviceID; - THROW_IF_FAILED(hr, "Audio Device Manager: Enumerator get device with ID: %S failed", device->getDescriptor().c_str()); - - hr = pDevice->QueryInterface(__uuidof(IMMEndpoint), (void**)&endpoint); - THROW_IF_FAILED(hr, "Audio Device Manager: Device get endpoint failed"); - - hr = pDevice->OpenPropertyStore(STGM_READ, &propertyStore); - THROW_IF_FAILED(hr, "Audio Device Manager: Device open property store failed"); - - hr = propertyStore->GetValue(PKEY_AudioEndpoint_FormFactor, &ff); - THROW_IF_FAILED(hr, "Audio Device Manager: PropertyStore get form factor property name failed"); + // The transport and form factor are optional details: an endpoint that cannot be + // read (one that has just gone away) keeps the unknown ones, so that it does not + // fail the enumeration of all the other devices. + if (SUCCEEDED(hr)) { + hr = pDevice->QueryInterface(__uuidof(IMMEndpoint), (void**)&endpoint); + } + if (SUCCEEDED(hr)) { + hr = pDevice->OpenPropertyStore(STGM_READ, &propertyStore); + } + if (SUCCEEDED(hr)) { + hr = propertyStore->GetValue(PKEY_AudioEndpoint_FormFactor, &ff); + } + if (FAILED(hr)) { + RTC_LOG(LS_WARNING) << "MMF: Get form factor of device " << device->getDescriptor() << " failed: HRESULT = " << hr; + return; + } EndpointFormFactor formFactor = static_cast(ff.uintVal); PropVariantClear(&ff); @@ -223,6 +229,12 @@ namespace jni PropVariantInit(&pv); HRESULT hr = deviceEnumerator->GetDefaultAudioEndpoint(dataFlow, eMultimedia, &defaultDevice); + + // No device of this kind, so no default either. + if (hr == HRESULT_FROM_WIN32(ERROR_NOT_FOUND)) { + return nullptr; + } + THROW_IF_FAILED(hr, "MMF: Get default audio endpoint failed"); hr = defaultDevice->GetId(&defaultDeviceId); From 7c096b2663f6ca38f3e2631ba613ef4317c80fde Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Sat, 10 Oct 2026 00:08:19 +0200 Subject: [PATCH 3/6] fix: report and clear what a Java device change listener throws 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. --- .../cpp/include/media/DeviceChangeListener.h | 3 + .../cpp/src/media/DeviceChangeListener.cpp | 63 +++++++++++-------- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/webrtc-jni/src/main/cpp/include/media/DeviceChangeListener.h b/webrtc-jni/src/main/cpp/include/media/DeviceChangeListener.h index 65a5a9f6..d3230b48 100644 --- a/webrtc-jni/src/main/cpp/include/media/DeviceChangeListener.h +++ b/webrtc-jni/src/main/cpp/include/media/DeviceChangeListener.h @@ -38,6 +38,9 @@ namespace jni void deviceConnected(avdev::DevicePtr device) override; void deviceDisconnected(avdev::DevicePtr device) override; + private: + void notify(const avdev::DevicePtr & device, jmethodID method); + private: class JavaDeviceChangeListenerClass : public JavaClass { diff --git a/webrtc-jni/src/main/cpp/src/media/DeviceChangeListener.cpp b/webrtc-jni/src/main/cpp/src/media/DeviceChangeListener.cpp index a7abaa7a..a452644c 100644 --- a/webrtc-jni/src/main/cpp/src/media/DeviceChangeListener.cpp +++ b/webrtc-jni/src/main/cpp/src/media/DeviceChangeListener.cpp @@ -22,6 +22,10 @@ #include "JavaUtils.h" #include "JNI_WebRTC.h" +#include "rtc_base/logging.h" + +#include + namespace jni { DeviceChangeListener::DeviceChangeListener(JNIEnv * env, const JavaGlobalRef & listener) : @@ -32,28 +36,15 @@ namespace jni void DeviceChangeListener::deviceConnected(avdev::DevicePtr device) { - JNIEnv * env = AttachCurrentThread(); - - if (env == nullptr) { - return; - } - - JavaLocalRef jdevice = nullptr; - - if (dynamic_cast(device.get())) { - jdevice = AudioDevice::toJavaAudioDevice(env, device); - } - else if (dynamic_cast(device.get())) { - const auto dev = dynamic_cast(device.get()); - jdevice = VideoDevice::toJavaVideoDevice(env, *dev); - } - - if (jdevice) { - env->CallVoidMethod(listener, javaClass->deviceConnected, jdevice.get()); - } + notify(device, javaClass->deviceConnected); } void DeviceChangeListener::deviceDisconnected(avdev::DevicePtr device) + { + notify(device, javaClass->deviceDisconnected); + } + + void DeviceChangeListener::notify(const avdev::DevicePtr & device, jmethodID method) { JNIEnv * env = AttachCurrentThread(); @@ -61,18 +52,36 @@ namespace jni return; } - JavaLocalRef jdevice = nullptr; + // This runs on a thread the operating system owns (a CoreAudio queue, the MMDevice + // notification thread, the PulseAudio main loop, ...), which has no handler for a C++ + // exception and never returns to Java. So nothing may be thrown from here, and what the + // Java listener throws is reported and cleared: left pending, it would be there for the + // next JNI call on this thread, which may be the next listener or the next event. + try { + JavaLocalRef jdevice = nullptr; - if (dynamic_cast(device.get())) { - jdevice = AudioDevice::toJavaAudioDevice(env, device); + if (dynamic_cast(device.get())) { + jdevice = AudioDevice::toJavaAudioDevice(env, device); + } + else if (dynamic_cast(device.get())) { + const auto dev = dynamic_cast(device.get()); + jdevice = VideoDevice::toJavaVideoDevice(env, *dev); + } + + if (jdevice) { + env->CallVoidMethod(listener, method, jdevice.get()); + } + } + catch (const std::exception & e) { + RTC_LOG(LS_ERROR) << "Device change notification failed: " << e.what(); } - else if (dynamic_cast(device.get())) { - const auto dev = dynamic_cast(device.get()); - jdevice = VideoDevice::toJavaVideoDevice(env, *dev); + catch (...) { + RTC_LOG(LS_ERROR) << "Device change notification failed"; } - if (jdevice) { - env->CallVoidMethod(listener, javaClass->deviceDisconnected, jdevice.get()); + if (env->ExceptionCheck()) { + env->ExceptionDescribe(); + env->ExceptionClear(); } } From 1b9a72242e5382be53dc5a55a17e9e7737ff88f7 Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Sat, 10 Oct 2026 00:08:20 +0200 Subject: [PATCH 4/6] fix: guard the device change listener list against concurrent use 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. --- .../main/cpp/include/media/DeviceManager.h | 2 + .../src/main/cpp/src/media/DeviceManager.cpp | 38 ++++++++++++++----- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/webrtc-jni/src/main/cpp/include/media/DeviceManager.h b/webrtc-jni/src/main/cpp/include/media/DeviceManager.h index a4482ff2..ce8bef38 100644 --- a/webrtc-jni/src/main/cpp/include/media/DeviceManager.h +++ b/webrtc-jni/src/main/cpp/include/media/DeviceManager.h @@ -19,6 +19,7 @@ #include #include +#include #include "media/DeviceHotplugListener.h" @@ -48,6 +49,7 @@ namespace jni void notifyListeners(DevicePtr device, const DeviceEvent event); std::list> hotplugListeners; + std::mutex listenerMutex; }; } } diff --git a/webrtc-jni/src/main/cpp/src/media/DeviceManager.cpp b/webrtc-jni/src/main/cpp/src/media/DeviceManager.cpp index ed1be519..e689d522 100644 --- a/webrtc-jni/src/main/cpp/src/media/DeviceManager.cpp +++ b/webrtc-jni/src/main/cpp/src/media/DeviceManager.cpp @@ -16,17 +16,23 @@ #include "media/DeviceManager.h" +#include + namespace jni { namespace avdev { void DeviceManager::attachHotplugListener(PDeviceHotplugListener listener) { + std::lock_guard lock(listenerMutex); + hotplugListeners.push_back(listener); } void DeviceManager::detachHotplugListener(PDeviceHotplugListener listener) { + std::lock_guard lock(listenerMutex); + hotplugListeners.remove_if([listener](std::weak_ptr p) { return !(p.owner_before(listener) || listener.owner_before(p)); }); @@ -44,21 +50,33 @@ namespace jni void DeviceManager::notifyListeners(DevicePtr device, const DeviceEvent event) { - for (auto i = hotplugListeners.begin(); i != hotplugListeners.end();) { - if ((*i).expired()) { - i = hotplugListeners.erase(i); - } - else { + // Listeners are attached and detached on Java threads while the operating system + // reports device changes on its own. The listeners are called on a copy, outside the + // lock, so that one may detach itself (or another) from within its callback. + std::vector listeners; + + { + std::lock_guard lock(listenerMutex); + + for (auto i = hotplugListeners.begin(); i != hotplugListeners.end();) { PDeviceHotplugListener listener = (*i).lock(); - if (event == DeviceEvent::Connected) { - listener->deviceConnected(device); + if (listener) { + listeners.push_back(listener); + ++i; } - else if (event == DeviceEvent::Disconnected) { - listener->deviceDisconnected(device); + else { + i = hotplugListeners.erase(i); } + } + } - ++i; + for (const PDeviceHotplugListener & listener : listeners) { + if (event == DeviceEvent::Connected) { + listener->deviceConnected(device); + } + else if (event == DeviceEvent::Disconnected) { + listener->deviceDisconnected(device); } } } From b6eb9728ee4c0dcb964b2a741844ed7ece17b9ed Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Sat, 10 Oct 2026 00:08:21 +0200 Subject: [PATCH 5/6] fix: delete a removed listener's global reference once, and lock the 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". --- .../jni-voithos/include/JavaContext.h | 11 ++- .../jni-voithos/src/JavaContext.cpp | 3 + .../webrtc/media/audio/AudioTrackTests.java | 74 +++++++++++++++++++ 3 files changed, 84 insertions(+), 4 deletions(-) diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaContext.h b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaContext.h index 724ecaed..b3a4700d 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaContext.h +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaContext.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -45,26 +46,26 @@ namespace jni std::shared_ptr removeNativeRef(JNIEnv * env, const JavaLocalRef & javaRef) { auto className = JavaClassUtils::toNativeClassName(env, javaRef); + + std::lock_guard lock(objectMapMutex); + auto it = objectMap.find(className); if (it == objectMap.end()) { return nullptr; } - auto globalRef = JavaGlobalRef(env, javaRef.get()); auto & list = it->second; std::shared_ptr nativeRef = nullptr; for (auto it = list.begin(); it != list.end(); ++it) { - if (env->IsSameObject(it->first, globalRef)) { + if (env->IsSameObject(it->first, javaRef)) { nativeRef = std::static_pointer_cast(it->second); list.erase(it); break; } } - env->DeleteGlobalRef(globalRef); - return nativeRef; } @@ -72,6 +73,8 @@ namespace jni JavaVM * vm; // Java object class name mapped to a Java object reference and its native object reference. std::unordered_map, std::shared_ptr>>> objectMap; + // Listeners are added and removed on any Java thread. + std::mutex objectMapMutex; }; } diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaContext.cpp b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaContext.cpp index 0f3813b0..20d9e6db 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaContext.cpp +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaContext.cpp @@ -24,6 +24,9 @@ namespace jni auto className = JavaClassUtils::toNativeClassName(env, javaRef); auto globalRef = JavaGlobalRef(env, javaRef.get()); auto pair = std::make_pair(globalRef, nativeRef); + + std::lock_guard lock(objectMapMutex); + auto it = objectMap.find(className); if (it == objectMap.end()) { diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioTrackTests.java b/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioTrackTests.java index 961b5377..9fd07213 100644 --- a/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioTrackTests.java +++ b/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioTrackTests.java @@ -19,6 +19,14 @@ import static org.junit.jupiter.api.Assertions.*; import dev.onvoid.webrtc.TestBase; +import dev.onvoid.webrtc.media.MediaStreamTrack; +import dev.onvoid.webrtc.media.MediaStreamTrackEndedListener; +import dev.onvoid.webrtc.media.MediaStreamTrackMuteListener; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CountDownLatch; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -71,4 +79,70 @@ void addRemoveSink() { audioTrack.removeSink(sink); } + @Test + void addRemoveListeners() { + MediaStreamTrackMuteListener muteListener = (track, muted) -> { }; + MediaStreamTrackEndedListener endedListener = track -> { }; + + audioTrack.addTrackMuteListener(muteListener); + audioTrack.addTrackEndedListener(endedListener); + + audioTrack.removeTrackMuteListener(muteListener); + audioTrack.removeTrackEndedListener(endedListener); + } + + @Test + void addRemoveListenersConcurrently() throws Exception { + // Each thread has a track of its own and new listeners (a lambda without + // captures would be one shared instance), so that the only state the + // threads share is the native registry of Java listeners. + final int threadCount = 8; + List tracks = new ArrayList<>(); + + for (int i = 0; i < threadCount; i++) { + AudioTrackSource source = factory.createAudioSource(new AudioOptions()); + tracks.add(factory.createAudioTrack("audioTrack" + i, source)); + } + + CountDownLatch start = new CountDownLatch(1); + List errors = Collections.synchronizedList(new ArrayList<>()); + List threads = new ArrayList<>(); + + for (AudioTrack track : tracks) { + Thread thread = new Thread(() -> { + try { + start.await(); + + for (int i = 0; i < 200; i++) { + MediaStreamTrackEndedListener listener = new MediaStreamTrackEndedListener() { + + @Override + public void onTrackEnd(MediaStreamTrack t) { + } + }; + + track.addTrackEndedListener(listener); + track.removeTrackEndedListener(listener); + } + } + catch (Throwable e) { + errors.add(e); + } + }); + thread.start(); + threads.add(thread); + } + + start.countDown(); + + for (Thread thread : threads) { + thread.join(); + } + for (AudioTrack track : tracks) { + track.dispose(); + } + + assertTrue(errors.isEmpty(), () -> "Errors: " + errors); + } + } From b460a725261cd2270c9a95bbd6a4cad44622ea1f Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Sat, 10 Oct 2026 00:21:22 +0200 Subject: [PATCH 6/6] fix: find a track listener by its track and kind, not by the listener 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. --- .../jni-voithos/include/JavaContext.h | 53 +++++++++++++-- .../jni-voithos/src/JavaContext.cpp | 16 +---- .../src/main/cpp/src/JNI_MediaStreamTrack.cpp | 16 +++-- .../webrtc/media/audio/AudioTrackTests.java | 65 +++++++++++++++++++ 4 files changed, 127 insertions(+), 23 deletions(-) diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaContext.h b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaContext.h index b3a4700d..41bfb400 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaContext.h +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaContext.h @@ -19,6 +19,7 @@ #include #include #include +#include namespace jni { @@ -40,10 +41,16 @@ namespace jni JavaVM * getVM(); - void addNativeRef(JNIEnv * env, const JavaLocalRef & javaRef, const std::shared_ptr & nativeRef); + // Keeps a native object (a listener's native peer) alive for as long as + // the Java object stays registered. An entry is found again by the Java + // object together with its owner and kind: the same Java listener can be + // registered with several owners (tracks), or as several kinds of + // listener with one owner, and each registration has its own native + // object. + void addNativeRef(JNIEnv * env, const JavaLocalRef & javaRef, const std::shared_ptr & nativeRef, const void * owner = nullptr, int kind = 0); template - std::shared_ptr removeNativeRef(JNIEnv * env, const JavaLocalRef & javaRef) + std::shared_ptr removeNativeRef(JNIEnv * env, const JavaLocalRef & javaRef, const void * owner = nullptr, int kind = 0) { auto className = JavaClassUtils::toNativeClassName(env, javaRef); @@ -59,8 +66,8 @@ namespace jni std::shared_ptr nativeRef = nullptr; for (auto it = list.begin(); it != list.end(); ++it) { - if (env->IsSameObject(it->first, javaRef)) { - nativeRef = std::static_pointer_cast(it->second); + if (it->owner == owner && it->kind == kind && env->IsSameObject(it->javaRef, javaRef)) { + nativeRef = std::static_pointer_cast(it->nativeRef); list.erase(it); break; } @@ -69,10 +76,44 @@ namespace jni return nativeRef; } + // Removes every entry of the owner, of all kinds, which must all hold a + // native object of type T. + template + std::vector> removeNativeRefs(const void * owner) + { + std::vector> nativeRefs; + + std::lock_guard lock(objectMapMutex); + + for (auto & entry : objectMap) { + auto & list = entry.second; + + for (auto it = list.begin(); it != list.end();) { + if (it->owner == owner) { + nativeRefs.push_back(std::static_pointer_cast(it->nativeRef)); + it = list.erase(it); + } + else { + ++it; + } + } + } + + return nativeRefs; + } + private: + struct NativeRef + { + JavaGlobalRef javaRef; + const void * owner; + int kind; + std::shared_ptr nativeRef; + }; + JavaVM * vm; - // Java object class name mapped to a Java object reference and its native object reference. - std::unordered_map, std::shared_ptr>>> objectMap; + // Java object class name mapped to the registrations of Java objects of that class. + std::unordered_map> objectMap; // Listeners are added and removed on any Java thread. std::mutex objectMapMutex; }; diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaContext.cpp b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaContext.cpp index 20d9e6db..a1dffc58 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaContext.cpp +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaContext.cpp @@ -19,23 +19,13 @@ namespace jni return vm; } - void JavaContext::addNativeRef(JNIEnv * env, const JavaLocalRef & javaRef, const std::shared_ptr & nativeRef) + void JavaContext::addNativeRef(JNIEnv * env, const JavaLocalRef & javaRef, const std::shared_ptr & nativeRef, const void * owner, int kind) { auto className = JavaClassUtils::toNativeClassName(env, javaRef); - auto globalRef = JavaGlobalRef(env, javaRef.get()); - auto pair = std::make_pair(globalRef, nativeRef); + NativeRef entry = { JavaGlobalRef(env, javaRef.get()), owner, kind, nativeRef }; std::lock_guard lock(objectMapMutex); - auto it = objectMap.find(className); - - if (it == objectMap.end()) { - std::list, std::shared_ptr>> list = { pair }; - - objectMap[className] = list; - } - else { - it->second.push_back(pair); - } + objectMap[className].push_back(std::move(entry)); } } \ No newline at end of file diff --git a/webrtc-jni/src/main/cpp/src/JNI_MediaStreamTrack.cpp b/webrtc-jni/src/main/cpp/src/JNI_MediaStreamTrack.cpp index e23695df..1ab3bf3e 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_MediaStreamTrack.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_MediaStreamTrack.cpp @@ -33,6 +33,14 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_MediaStreamTrack_dispose webrtc::MediaStreamTrackInterface * track = GetHandle(env, caller); CHECK_HANDLE(track); + // Listeners still registered are taken off the track before it goes. Their + // entries are keyed by this track's address, which a later track can be + // given, and an observer left registered would be freed with its entry + // while the track (if something else still holds it) can still call it. + for (const auto & observer : javaContext->removeNativeRefs(track)) { + track->UnregisterObserver(observer.get()); + } + webrtc::RefCountReleaseStatus status = track->Release(); // Our reference is gone the moment Release() returns, regardless of @@ -104,7 +112,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_MediaStreamTrack_addEndedEve track->RegisterObserver(observer); - javaContext->addNativeRef(env, jni::JavaLocalRef(env, jListener), observerPtr); + javaContext->addNativeRef(env, jni::JavaLocalRef(env, jListener), observerPtr, track, static_cast(jni::MediaStreamTrackEvent::ended)); } catch (...) { ThrowCxxJavaException(env); @@ -118,7 +126,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_MediaStreamTrack_removeEnded CHECK_HANDLE(track); try { - auto observerPtr = javaContext->removeNativeRef(env, jni::JavaLocalRef(env, jListener)); + auto observerPtr = javaContext->removeNativeRef(env, jni::JavaLocalRef(env, jListener), track, static_cast(jni::MediaStreamTrackEvent::ended)); if (observerPtr) { track->UnregisterObserver(observerPtr.get()); @@ -141,7 +149,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_MediaStreamTrack_addMuteEven track->RegisterObserver(observer); - javaContext->addNativeRef(env, jni::JavaLocalRef(env, jListener), observerPtr); + javaContext->addNativeRef(env, jni::JavaLocalRef(env, jListener), observerPtr, track, static_cast(jni::MediaStreamTrackEvent::mute)); } catch (...) { ThrowCxxJavaException(env); @@ -155,7 +163,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_MediaStreamTrack_removeMuteE CHECK_HANDLE(track); try { - auto observerPtr = javaContext->removeNativeRef(env, jni::JavaLocalRef(env, jListener)); + auto observerPtr = javaContext->removeNativeRef(env, jni::JavaLocalRef(env, jListener), track, static_cast(jni::MediaStreamTrackEvent::mute)); if (observerPtr) { track->UnregisterObserver(observerPtr.get()); diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioTrackTests.java b/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioTrackTests.java index 9fd07213..f7b68948 100644 --- a/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioTrackTests.java +++ b/webrtc/src/test/java/dev/onvoid/webrtc/media/audio/AudioTrackTests.java @@ -91,6 +91,71 @@ void addRemoveListeners() { audioTrack.removeTrackEndedListener(endedListener); } + @Test + void removeListenerFromOneOfTwoTracks() { + // Mute events are delivered before setEnabled() returns. + AudioTrack otherTrack = factory.createAudioTrack("otherTrack", + factory.createAudioSource(new AudioOptions())); + List mutedTracks = Collections.synchronizedList(new ArrayList<>()); + MediaStreamTrackMuteListener listener = (track, muted) -> mutedTracks.add(track.getId()); + + audioTrack.addTrackMuteListener(listener); + otherTrack.addTrackMuteListener(listener); + + // Removing the second registration must not take the first one. + otherTrack.removeTrackMuteListener(listener); + + otherTrack.setEnabled(false); + audioTrack.setEnabled(false); + + audioTrack.removeTrackMuteListener(listener); + otherTrack.dispose(); + + assertEquals(Collections.singletonList("audioTrack"), mutedTracks); + } + + @Test + void removeListenerOfOneKind() { + // One object registered as both a mute and an ended listener. + List muteEvents = Collections.synchronizedList(new ArrayList<>()); + + class Listener implements MediaStreamTrackMuteListener, MediaStreamTrackEndedListener { + + @Override + public void onTrackMute(MediaStreamTrack track, boolean muted) { + muteEvents.add(muted); + } + + @Override + public void onTrackEnd(MediaStreamTrack track) { + } + } + + Listener listener = new Listener(); + + audioTrack.addTrackMuteListener(listener); + audioTrack.addTrackEndedListener(listener); + + audioTrack.removeTrackEndedListener(listener); + + audioTrack.setEnabled(false); + + audioTrack.removeTrackMuteListener(listener); + + assertEquals(Collections.singletonList(true), muteEvents); + } + + @Test + void disposeWithListeners() { + AudioTrack track = factory.createAudioTrack("otherTrack", + factory.createAudioSource(new AudioOptions())); + + track.addTrackMuteListener((t, muted) -> { }); + track.addTrackEndedListener(t -> { }); + + assertDoesNotThrow(track::dispose); + } + @Test void addRemoveListenersConcurrently() throws Exception { // Each thread has a track of its own and new listeners (a lambda without