admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix NetworkMonitor race condition when dispatching native observers

Browse Source 
There is a race condition in NetworkMonitor where native observers may be removed concurrently with a notification being dispatched, leading to a dangling pointer dereference (trying to dispatch an observer that was already removed and destroyed), and from there a crash with access violation.

By ensuring dispatching to native observers is done within the synchronization lock that guards additions/removals of native observers protects against this race condition.
Since native observers callbacks are posted to the networking thread in the C++ side anyway, there should be no risk of deadlock/starvation due to long-running observers.

Bug: webrtc:15837
Change-Id: Id2b788f102dbd25de76ceed434c4cd68aa9a569e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/338643
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42256}
This commit is contained in:
Guy Hershenbaum 2024-02-08 08:09:06 -08:00 committed by WebRTC LUCI CQ
parent fc57037462
commit 94dfe1cc59
1 changed files with 19 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
sdk/android/api/org/webrtc/NetworkMonitor.java
View File

@ -227,10 +227,13 @@ public class NetworkMonitor {
/** Alerts all observers of a connection change. */
private void notifyObserversOfConnectionTypeChange(
NetworkChangeDetector.ConnectionType newConnectionType) {
List<Long> nativeObservers = getNativeNetworkObserversSync();
for (Long nativeObserver : nativeObservers) {
nativeNotifyConnectionTypeChanged(nativeObserver);
synchronized (nativeNetworkObservers) {
for (Long nativeObserver : nativeNetworkObservers) {
nativeNotifyConnectionTypeChanged(nativeObserver);
}
}
// This avoids calling external methods while locking on an object.
List<NetworkObserver> javaObservers;
synchronized (networkObservers) {
@ -243,25 +246,28 @@ public class NetworkMonitor {
private void notifyObserversOfNetworkConnect(
NetworkChangeDetector.NetworkInformation networkInfo) {
List<Long> nativeObservers = getNativeNetworkObserversSync();
for (Long nativeObserver : nativeObservers) {
nativeNotifyOfNetworkConnect(nativeObserver, networkInfo);
synchronized (nativeNetworkObservers) {
for (Long nativeObserver : nativeNetworkObservers) {
nativeNotifyOfNetworkConnect(nativeObserver, networkInfo);
}
}
}
private void notifyObserversOfNetworkDisconnect(long networkHandle) {
List<Long> nativeObservers = getNativeNetworkObserversSync();
for (Long nativeObserver : nativeObservers) {
nativeNotifyOfNetworkDisconnect(nativeObserver, networkHandle);
synchronized (nativeNetworkObservers) {
for (Long nativeObserver : nativeNetworkObservers) {
nativeNotifyOfNetworkDisconnect(nativeObserver, networkHandle);
}
}
}
private void notifyObserversOfNetworkPreference(
List<NetworkChangeDetector.ConnectionType> types, int preference) {
List<Long> nativeObservers = getNativeNetworkObserversSync();
for (NetworkChangeDetector.ConnectionType type : types) {
for (Long nativeObserver : nativeObservers) {
nativeNotifyOfNetworkPreference(nativeObserver, type, preference);
synchronized(nativeNetworkObservers) {
for (NetworkChangeDetector.ConnectionType type : types) {
for (Long nativeObserver : nativeNetworkObservers) {
nativeNotifyOfNetworkPreference(nativeObserver, type, preference);
}
}
}
}
@ -282,12 +288,6 @@ public class NetworkMonitor {
nativeNotifyOfActiveNetworkList(nativeObserver, networkInfos);
}
private List<Long> getNativeNetworkObserversSync() {
synchronized (nativeNetworkObservers) {
return new ArrayList<>(nativeNetworkObservers);
}
}
/**
* Adds an observer for any connection type changes.
*