AndroidNetworkMonitor - loosen assumptions even more

This cl/ attempts to fix (rather) rare crashes in
OnNetworkConnected_n by loosening the assumptions
that a network handle will keep it's network name.

With this cl/ it is possible that a NetworkHandle
can call OnNetworkConnected_n with one interface name
and then directly afterwards call it with another (
w/o an OnNetworkDisconnected_n inbetween).

This is the only scenario in which I could see the previous
crash occurring.

i.e
OnNetworkConnected(handle, "some-if-name")
OnNetworkConnected(handle, "some-other-name")

- previously this caused crash,
- now this is treated as if there was an OnNetworkDisconnected(handle) in between.

---

Also 1: shamelessly copy TYPE_MOBILE_DUN & TYPE_MOBILE_HIPRI from chromium: 87987f0e76

Also 2: Modify testcase not to use real interface names, so I can ran them on personal test phone w/o the real networks interfering.

Bug: webrtc:13741
Change-Id: I5480d5ce7031c2b5c09b958064076d02b3db1248
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/285980
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38808}
This commit is contained in:
Jonas Oreland
2022-12-05 10:40:09 +01:00
committed by WebRTC LUCI CQ
parent b6e8c2e393
commit c0c65387ae
3 changed files with 128 additions and 54 deletions

View File

@ -104,7 +104,10 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo
@Override
public void onAvailable(Network network) {
Logging.d(TAG, "Network becomes available: " + network.toString());
Logging.d(TAG,
"Network"
+ " handle: " + networkToNetId(network)
+ " becomes available: " + network.toString());
synchronized (availableNetworks) {
availableNetworks.add(network);
@ -116,7 +119,9 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo
public void onCapabilitiesChanged(Network network, NetworkCapabilities networkCapabilities) {
// A capabilities change may indicate the ConnectionType has changed,
// so forward the new NetworkInformation along to the observer.
Logging.d(TAG, "capabilities changed: " + networkCapabilities.toString());
Logging.d(TAG,
"handle: " + networkToNetId(network)
+ " capabilities changed: " + networkCapabilities.toString());
onNetworkChanged(network);
}
@ -127,7 +132,7 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo
//
// linkProperties.toString() has PII that cannot be redacted
// very reliably, so do not include in log.
Logging.d(TAG, "link properties changed");
Logging.d(TAG, "handle: " + networkToNetId(network) + " link properties changed");
onNetworkChanged(network);
}
@ -135,13 +140,18 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo
public void onLosing(Network network, int maxMsToLive) {
// Tell the network is going to lose in MaxMsToLive milliseconds.
// We may use this signal later.
Logging.d(
TAG, "Network " + network.toString() + " is about to lose in " + maxMsToLive + "ms");
Logging.d(TAG,
"Network"
+ " handle: " + networkToNetId(network) + ", " + network.toString()
+ " is about to lose in " + maxMsToLive + "ms");
}
@Override
public void onLost(Network network) {
Logging.d(TAG, "Network " + network.toString() + " is disconnected");
Logging.d(TAG,
"Network"
+ " handle: " + networkToNetId(network) + ", " + network.toString()
+ " is disconnected");
synchronized (availableNetworks) {
availableNetworks.remove(network);
@ -789,6 +799,8 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo
case ConnectivityManager.TYPE_BLUETOOTH:
return NetworkChangeDetector.ConnectionType.CONNECTION_BLUETOOTH;
case ConnectivityManager.TYPE_MOBILE:
case ConnectivityManager.TYPE_MOBILE_DUN:
case ConnectivityManager.TYPE_MOBILE_HIPRI:
// Use information from TelephonyManager to classify the connection.
switch (networkSubtype) {
case TelephonyManager.NETWORK_TYPE_GPRS:

View File

@ -25,6 +25,10 @@ static const uint32_t kTestIpv4Address = 0xC0A80011; // 192.168.0.17
static const char kTestIpv6Address1[] = "2a00:8a00:a000:1190:0000:0001:000:252";
static const char kTestIpv6Address2[] = "2a00:8a00:a000:1190:0000:0002:000:253";
static const char kTestIfName1[] = "testlan0";
static const char kTestIfName1V4[] = "v4-testlan0";
static const char kTestIfName2[] = "testnet0";
jni::NetworkInformation CreateNetworkInformation(
const std::string& interface_name,
jni::NetworkHandle network_handle,
@ -76,7 +80,7 @@ TEST_F(AndroidNetworkMonitorTest, TestFindNetworkHandleUsingIpv4Address) {
jni::NetworkHandle ipv4_handle = 100;
rtc::IPAddress ipv4_address(kTestIpv4Address);
jni::NetworkInformation net_info =
CreateNetworkInformation("wlan0", ipv4_handle, ipv4_address);
CreateNetworkInformation(kTestIfName1, ipv4_handle, ipv4_address);
std::vector<jni::NetworkInformation> net_infos(1, net_info);
network_monitor_->SetNetworkInfos(net_infos);
@ -93,9 +97,9 @@ TEST_F(AndroidNetworkMonitorTest, TestFindNetworkHandleUsingFullIpv6Address) {
rtc::IPAddress ipv6_address2 = GetIpAddressFromIpv6String(kTestIpv6Address2);
// Set up an IPv6 network.
jni::NetworkInformation net_info =
CreateNetworkInformation("wlan0", ipv6_handle, ipv6_address1);
CreateNetworkInformation(kTestIfName1, ipv6_handle, ipv6_address1);
std::vector<jni::NetworkInformation> net_infos(1, net_info);
network_monitor_->SetNetworkInfos(net_infos);
network_monitor_->OnNetworkConnected_n(net_info);
auto network_handle1 =
network_monitor_->FindNetworkHandleFromAddressOrName(ipv6_address1, "");
@ -119,7 +123,7 @@ TEST_F(AndroidNetworkMonitorTest,
rtc::IPAddress ipv6_address2 = GetIpAddressFromIpv6String(kTestIpv6Address2);
// Set up an IPv6 network.
jni::NetworkInformation net_info =
CreateNetworkInformation("wlan0", ipv6_handle, ipv6_address1);
CreateNetworkInformation(kTestIfName1, ipv6_handle, ipv6_address1);
std::vector<jni::NetworkInformation> net_infos(1, net_info);
network_monitor_->OnNetworkConnected_n(net_info);
@ -142,7 +146,7 @@ TEST_F(AndroidNetworkMonitorTest, TestFindNetworkHandleUsingIfName) {
// Set up an IPv6 network.
jni::NetworkInformation net_info =
CreateNetworkInformation("wlan0", ipv6_handle, ipv6_address1);
CreateNetworkInformation(kTestIfName1, ipv6_handle, ipv6_address1);
std::vector<jni::NetworkInformation> net_infos(1, net_info);
network_monitor_->OnNetworkConnected_n(net_info);
@ -154,7 +158,7 @@ TEST_F(AndroidNetworkMonitorTest, TestFindNetworkHandleUsingIfName) {
// Search using ip address AND if_name (for typical ipv4 over ipv6 tunnel).
auto network_handle2 = network_monitor_->FindNetworkHandleFromAddressOrName(
ipv4_address, "v4-wlan0");
ipv4_address, kTestIfName1V4);
ASSERT_FALSE(network_handle1.has_value());
ASSERT_TRUE(network_handle2.has_value());
@ -167,14 +171,14 @@ TEST_F(AndroidNetworkMonitorTest, TestUnderlyingVpnType) {
jni::NetworkHandle ipv4_handle = 100;
rtc::IPAddress ipv4_address(kTestIpv4Address);
jni::NetworkInformation net_info =
CreateNetworkInformation("wlan0", ipv4_handle, ipv4_address);
CreateNetworkInformation(kTestIfName1, ipv4_handle, ipv4_address);
net_info.type = jni::NETWORK_VPN;
net_info.underlying_type_for_vpn = jni::NETWORK_WIFI;
network_monitor_->OnNetworkConnected_n(net_info);
EXPECT_EQ(
rtc::ADAPTER_TYPE_WIFI,
network_monitor_->GetInterfaceInfo("v4-wlan0").underlying_type_for_vpn);
EXPECT_EQ(rtc::ADAPTER_TYPE_WIFI,
network_monitor_->GetInterfaceInfo(kTestIfName1V4)
.underlying_type_for_vpn);
}
// Verify that Disconnect makes interface unavailable.
@ -184,25 +188,26 @@ TEST_F(AndroidNetworkMonitorTest, Disconnect) {
jni::NetworkHandle ipv4_handle = 100;
rtc::IPAddress ipv4_address(kTestIpv4Address);
jni::NetworkInformation net_info =
CreateNetworkInformation("wlan0", ipv4_handle, ipv4_address);
CreateNetworkInformation(kTestIfName1, ipv4_handle, ipv4_address);
net_info.type = jni::NETWORK_WIFI;
network_monitor_->OnNetworkConnected_n(net_info);
EXPECT_TRUE(network_monitor_->GetInterfaceInfo("wlan0").available);
EXPECT_TRUE(network_monitor_
->FindNetworkHandleFromAddressOrName(ipv4_address, "v4-wlan0")
.has_value());
EXPECT_EQ(network_monitor_->GetInterfaceInfo("v4-wlan0").adapter_type,
EXPECT_TRUE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_TRUE(
network_monitor_
->FindNetworkHandleFromAddressOrName(ipv4_address, kTestIfName1V4)
.has_value());
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1V4).adapter_type,
rtc::ADAPTER_TYPE_WIFI);
// Check that values are reset on disconnect().
Disconnect(ipv4_handle);
EXPECT_FALSE(network_monitor_->GetInterfaceInfo("wlan0").available);
EXPECT_FALSE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_FALSE(
network_monitor_
->FindNetworkHandleFromAddressOrName(ipv4_address, "v4-wlan0")
->FindNetworkHandleFromAddressOrName(ipv4_address, kTestIfName1V4)
.has_value());
EXPECT_EQ(network_monitor_->GetInterfaceInfo("v4-wlan0").adapter_type,
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1V4).adapter_type,
rtc::ADAPTER_TYPE_UNKNOWN);
}
@ -213,25 +218,26 @@ TEST_F(AndroidNetworkMonitorTest, Reset) {
jni::NetworkHandle ipv4_handle = 100;
rtc::IPAddress ipv4_address(kTestIpv4Address);
jni::NetworkInformation net_info =
CreateNetworkInformation("wlan0", ipv4_handle, ipv4_address);
CreateNetworkInformation(kTestIfName1, ipv4_handle, ipv4_address);
net_info.type = jni::NETWORK_WIFI;
network_monitor_->OnNetworkConnected_n(net_info);
EXPECT_TRUE(network_monitor_->GetInterfaceInfo("wlan0").available);
EXPECT_TRUE(network_monitor_
->FindNetworkHandleFromAddressOrName(ipv4_address, "v4-wlan0")
.has_value());
EXPECT_EQ(network_monitor_->GetInterfaceInfo("v4-wlan0").adapter_type,
EXPECT_TRUE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_TRUE(
network_monitor_
->FindNetworkHandleFromAddressOrName(ipv4_address, kTestIfName1V4)
.has_value());
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1V4).adapter_type,
rtc::ADAPTER_TYPE_WIFI);
// Check that values are reset on Stop().
network_monitor_->Stop();
EXPECT_FALSE(network_monitor_->GetInterfaceInfo("wlan0").available);
EXPECT_FALSE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_FALSE(
network_monitor_
->FindNetworkHandleFromAddressOrName(ipv4_address, "v4-wlan0")
->FindNetworkHandleFromAddressOrName(ipv4_address, kTestIfName1V4)
.has_value());
EXPECT_EQ(network_monitor_->GetInterfaceInfo("v4-wlan0").adapter_type,
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1V4).adapter_type,
rtc::ADAPTER_TYPE_UNKNOWN);
}
@ -241,21 +247,21 @@ TEST_F(AndroidNetworkMonitorTest, DuplicateIfname) {
jni::NetworkHandle ipv4_handle = 100;
rtc::IPAddress ipv4_address(kTestIpv4Address);
jni::NetworkInformation net_info1 =
CreateNetworkInformation("wlan0", ipv4_handle, ipv4_address);
CreateNetworkInformation(kTestIfName1, ipv4_handle, ipv4_address);
net_info1.type = jni::NETWORK_WIFI;
jni::NetworkHandle ipv6_handle = 101;
rtc::IPAddress ipv6_address = GetIpAddressFromIpv6String(kTestIpv6Address1);
jni::NetworkInformation net_info2 =
CreateNetworkInformation("wlan0", ipv6_handle, ipv6_address);
CreateNetworkInformation(kTestIfName1, ipv6_handle, ipv6_address);
net_info2.type = jni::NETWORK_UNKNOWN_CELLULAR;
network_monitor_->OnNetworkConnected_n(net_info1);
network_monitor_->OnNetworkConnected_n(net_info2);
// The last added.
EXPECT_TRUE(network_monitor_->GetInterfaceInfo("wlan0").available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo("v-wlan0").adapter_type,
EXPECT_TRUE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1V4).adapter_type,
rtc::ADAPTER_TYPE_CELLULAR);
// But both IP addresses are still searchable.
@ -273,28 +279,28 @@ TEST_F(AndroidNetworkMonitorTest, DuplicateIfnameDisconnectOwner) {
jni::NetworkHandle ipv4_handle = 100;
rtc::IPAddress ipv4_address(kTestIpv4Address);
jni::NetworkInformation net_info1 =
CreateNetworkInformation("wlan0", ipv4_handle, ipv4_address);
CreateNetworkInformation(kTestIfName1, ipv4_handle, ipv4_address);
net_info1.type = jni::NETWORK_WIFI;
jni::NetworkHandle ipv6_handle = 101;
rtc::IPAddress ipv6_address = GetIpAddressFromIpv6String(kTestIpv6Address1);
jni::NetworkInformation net_info2 =
CreateNetworkInformation("wlan0", ipv6_handle, ipv6_address);
CreateNetworkInformation(kTestIfName1, ipv6_handle, ipv6_address);
net_info2.type = jni::NETWORK_UNKNOWN_CELLULAR;
network_monitor_->OnNetworkConnected_n(net_info1);
network_monitor_->OnNetworkConnected_n(net_info2);
// The last added.
EXPECT_TRUE(network_monitor_->GetInterfaceInfo("wlan0").available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo("v-wlan0").adapter_type,
EXPECT_TRUE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1V4).adapter_type,
rtc::ADAPTER_TYPE_CELLULAR);
Disconnect(ipv6_handle);
// We should now find ipv4_handle.
EXPECT_TRUE(network_monitor_->GetInterfaceInfo("wlan0").available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo("v-wlan0").adapter_type,
EXPECT_TRUE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1V4).adapter_type,
rtc::ADAPTER_TYPE_WIFI);
}
@ -304,30 +310,67 @@ TEST_F(AndroidNetworkMonitorTest, DuplicateIfnameDisconnectNonOwner) {
jni::NetworkHandle ipv4_handle = 100;
rtc::IPAddress ipv4_address(kTestIpv4Address);
jni::NetworkInformation net_info1 =
CreateNetworkInformation("wlan0", ipv4_handle, ipv4_address);
CreateNetworkInformation(kTestIfName1, ipv4_handle, ipv4_address);
net_info1.type = jni::NETWORK_WIFI;
jni::NetworkHandle ipv6_handle = 101;
rtc::IPAddress ipv6_address = GetIpAddressFromIpv6String(kTestIpv6Address1);
jni::NetworkInformation net_info2 =
CreateNetworkInformation("wlan0", ipv6_handle, ipv6_address);
CreateNetworkInformation(kTestIfName1, ipv6_handle, ipv6_address);
net_info2.type = jni::NETWORK_UNKNOWN_CELLULAR;
network_monitor_->OnNetworkConnected_n(net_info1);
network_monitor_->OnNetworkConnected_n(net_info2);
// The last added.
EXPECT_TRUE(network_monitor_->GetInterfaceInfo("wlan0").available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo("wlan0").adapter_type,
EXPECT_TRUE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1).adapter_type,
rtc::ADAPTER_TYPE_CELLULAR);
Disconnect(ipv4_handle);
// We should still find ipv6 network.
EXPECT_TRUE(network_monitor_->GetInterfaceInfo("wlan0").available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo("v-wlan0").adapter_type,
EXPECT_TRUE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1V4).adapter_type,
rtc::ADAPTER_TYPE_CELLULAR);
}
TEST_F(AndroidNetworkMonitorTest, ReconnectWithoutDisconnect) {
network_monitor_->Start();
jni::NetworkHandle ipv4_handle = 100;
rtc::IPAddress ipv4_address(kTestIpv4Address);
jni::NetworkInformation net_info1 =
CreateNetworkInformation(kTestIfName1, ipv4_handle, ipv4_address);
net_info1.type = jni::NETWORK_WIFI;
rtc::IPAddress ipv6_address = GetIpAddressFromIpv6String(kTestIpv6Address1);
jni::NetworkInformation net_info2 =
CreateNetworkInformation(kTestIfName2, ipv4_handle, ipv6_address);
net_info2.type = jni::NETWORK_UNKNOWN_CELLULAR;
network_monitor_->OnNetworkConnected_n(net_info1);
network_monitor_->OnNetworkConnected_n(net_info2);
// Only last one should still be there!
EXPECT_TRUE(network_monitor_->GetInterfaceInfo(kTestIfName2).available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName2).adapter_type,
rtc::ADAPTER_TYPE_CELLULAR);
EXPECT_FALSE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1).adapter_type,
rtc::ADAPTER_TYPE_UNKNOWN);
Disconnect(ipv4_handle);
// Should be empty!
EXPECT_FALSE(network_monitor_->GetInterfaceInfo(kTestIfName2).available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName2).adapter_type,
rtc::ADAPTER_TYPE_UNKNOWN);
EXPECT_FALSE(network_monitor_->GetInterfaceInfo(kTestIfName1).available);
EXPECT_EQ(network_monitor_->GetInterfaceInfo(kTestIfName1).adapter_type,
rtc::ADAPTER_TYPE_UNKNOWN);
}
} // namespace test
} // namespace webrtc

View File

@ -416,6 +416,25 @@ void AndroidNetworkMonitor::OnNetworkConnected_n(
const NetworkInformation& network_info) {
RTC_DCHECK_RUN_ON(network_thread_);
RTC_LOG(LS_INFO) << "Network connected: " << network_info.ToString();
// We speculate that OnNetworkConnected_n can be called with the same handle
// and different if_names. Handle this as if the network was first
// disconnected.
auto iter = network_info_by_handle_.find(network_info.handle);
if (iter != network_info_by_handle_.end()) {
// Remove old if_name for this handle if they don't match.
if (network_info.interface_name != iter->second.interface_name) {
RTC_LOG(LS_INFO) << "Network"
<< " handle " << network_info.handle
<< " change if_name from: "
<< iter->second.interface_name
<< " to: " << network_info.interface_name;
RTC_DCHECK(network_handle_by_if_name_[iter->second.interface_name] ==
network_info.handle);
network_handle_by_if_name_.erase(iter->second.interface_name);
}
}
network_info_by_handle_[network_info.handle] = network_info;
for (const rtc::IPAddress& address : network_info.ip_addresses) {
network_handle_by_address_[address] = network_info.handle;
@ -431,7 +450,6 @@ AndroidNetworkMonitor::FindNetworkHandleFromAddressOrName(
const rtc::IPAddress& ip_address,
absl::string_view if_name) const {
RTC_DCHECK_RUN_ON(network_thread_);
RTC_LOG(LS_INFO) << "Find network handle.";
if (find_network_handle_without_ipv6_temporary_part_) {
for (auto const& iter : network_info_by_handle_) {
const std::vector<rtc::IPAddress>& addresses = iter.second.ip_addresses;
@ -484,12 +502,13 @@ void AndroidNetworkMonitor::OnNetworkDisconnected_n(NetworkHandle handle) {
return;
}
for (const rtc::IPAddress& address : iter->second.ip_addresses) {
const auto& network_info = iter->second;
for (const rtc::IPAddress& address : network_info.ip_addresses) {
network_handle_by_address_.erase(address);
}
// We've discovered that the if_name is not always unique,
// i.e it can be several network conencted with same if_name.
// i.e it can be several network connected with same if_name.
//
// This is handled the following way,
// 1) OnNetworkConnected_n overwrites any previous "owner" of an interface
@ -501,7 +520,7 @@ void AndroidNetworkMonitor::OnNetworkDisconnected_n(NetworkHandle handle) {
// network_handle_by_if_name_.
// Check if we are registered as "owner" of if_name.
const auto& if_name = iter->second.interface_name;
const auto& if_name = network_info.interface_name;
auto iter2 = network_handle_by_if_name_.find(if_name);
RTC_DCHECK(iter2 != network_handle_by_if_name_.end());
if (iter2 != network_handle_by_if_name_.end() && iter2->second == handle) {