Rewrite "Reset all maps in AndroidNetworkMonitor Start()/Stop()"
This reverts commit 8cd7b0a7babf3f58f6beab70fcabeb75e66c1bed. The assumption in AndroidNetworkMonitor that an interface name is unique has turned out to be incorrect :( for some (weird) devices, i.e ccmni0. It is unclear if it is a permanent setup or a transient state. This cl/ changes the impl. to cope with that, the last OnNetworkConnected_n "owns" the interface name, and when OnNetworkDisconnected_n runs, we check if we're "owner" and maybe set a new "owner" (if we're not "owner" we do nothing). New testcases added. I also 1) change NetworkMonitorInterface to return a struct with all the information that is requested with interface name as key. 2) Change Network.cc adding (debug) assertions that network properties can't change inside a loop (in one thread). Original change's description: > Revert "Reset all maps in AndroidNetworkMonitor Start()/Stop()" > > This reverts commit 02293096f9689fee3d32defa77dca227cc1eee90. > > Reason for revert: mysterious crashes in android_network_monitor.cc > > Original change's description: > > Reset all maps in AndroidNetworkMonitor Start()/Stop() > > > > This cl/ fixes another race condition with the recent additions > > to NetworkMonitorAutoDetect (getAllNetworksFromCache). > > > > The getAllNetworksFromCache-feature uses the by the Android team > > preferred way of enumerating networks, i.e to register network listeners. > > > > Th recent fix to add IsAdapterAvailable, https://webrtc-review.googlesource.com/c/src/+/257400 > > contained a bug in that the adapter_type_by_name_ map was not > > reset either on disconnect or Start/Stop. > > > > This cl/ addresses that including unit test. > > It also de-obfuscates NetworkMonitor so that it always > > calls NotifyOfActiveNetworkList on startMonitoring even > > if list.size() == 0. This should not matter but makes > > code easier to understand. > > > > Bug: webrtc:13741 > > Change-Id: I438b877eebf769a8b2e7292b697ef1c0a349b24f > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/258721 > > Reviewed-by: Harald Alvestrand <hta@webrtc.org> > > Commit-Queue: Jonas Oreland <jonaso@webrtc.org> > > Cr-Commit-Position: refs/heads/main@{#36530} > > Bug: webrtc:13741 > Change-Id: I36fbf63f658d3e8048e13959cbebfbd14df12b14 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264146 > Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org> > Commit-Queue: Jonas Oreland <jonaso@webrtc.org> > Cr-Commit-Position: refs/heads/main@{#37016} Bug: webrtc:13741 Change-Id: Ib4eb072b775e493b564528f0be94c685b70ec20f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264421 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Jonas Oreland <jonaso@webrtc.org> Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org> Cr-Commit-Position: refs/heads/main@{#37056}
This commit is contained in:
committed by
WebRTC LUCI CQ
parent
6fb8d1a2d7
commit
61dbcd115a
@ -134,14 +134,17 @@ static rtc::AdapterType AdapterTypeFromNetworkType(
|
||||
case NETWORK_UNKNOWN_CELLULAR:
|
||||
return rtc::ADAPTER_TYPE_CELLULAR;
|
||||
case NETWORK_VPN:
|
||||
return rtc::ADAPTER_TYPE_VPN;
|
||||
case NETWORK_BLUETOOTH:
|
||||
// There is no corresponding mapping for bluetooth networks.
|
||||
// Map it to VPN for now.
|
||||
return rtc::ADAPTER_TYPE_VPN;
|
||||
default:
|
||||
RTC_DCHECK_NOTREACHED() << "Invalid network type " << network_type;
|
||||
// Map it to UNKNOWN for now.
|
||||
return rtc::ADAPTER_TYPE_UNKNOWN;
|
||||
case NETWORK_NONE:
|
||||
return rtc::ADAPTER_TYPE_UNKNOWN;
|
||||
}
|
||||
|
||||
RTC_DCHECK_NOTREACHED() << "Invalid network type " << network_type;
|
||||
return rtc::ADAPTER_TYPE_UNKNOWN;
|
||||
}
|
||||
|
||||
static rtc::IPAddress JavaToNativeIpAddress(
|
||||
@ -243,6 +246,7 @@ void AndroidNetworkMonitor::Start() {
|
||||
if (started_) {
|
||||
return;
|
||||
}
|
||||
reset();
|
||||
started_ = true;
|
||||
surface_cellular_types_ =
|
||||
field_trials_.IsEnabled("WebRTC-SurfaceCellularTypes");
|
||||
@ -265,6 +269,14 @@ void AndroidNetworkMonitor::Start() {
|
||||
env, field_trials_.Lookup("WebRTC-NetworkMonitorAutoDetect")));
|
||||
}
|
||||
|
||||
void AndroidNetworkMonitor::reset() {
|
||||
RTC_DCHECK_RUN_ON(network_thread_);
|
||||
network_handle_by_address_.clear();
|
||||
network_handle_by_if_name_.clear();
|
||||
network_info_by_handle_.clear();
|
||||
network_preference_by_adapter_type_.clear();
|
||||
}
|
||||
|
||||
void AndroidNetworkMonitor::Stop() {
|
||||
RTC_DCHECK_RUN_ON(network_thread_);
|
||||
if (!started_) {
|
||||
@ -281,8 +293,7 @@ void AndroidNetworkMonitor::Stop() {
|
||||
Java_NetworkMonitor_stopMonitoring(env, j_network_monitor_,
|
||||
jlongFromPointer(this));
|
||||
|
||||
network_handle_by_address_.clear();
|
||||
network_info_by_handle_.clear();
|
||||
reset();
|
||||
}
|
||||
|
||||
// The implementation is largely taken from UDPSocketPosix::BindToNetwork in
|
||||
@ -406,17 +417,13 @@ void AndroidNetworkMonitor::OnNetworkConnected_n(
|
||||
const NetworkInformation& network_info) {
|
||||
RTC_DCHECK_RUN_ON(network_thread_);
|
||||
RTC_LOG(LS_INFO) << "Network connected: " << network_info.ToString();
|
||||
adapter_type_by_name_[network_info.interface_name] =
|
||||
AdapterTypeFromNetworkType(network_info.type, surface_cellular_types_);
|
||||
if (network_info.type == NETWORK_VPN) {
|
||||
vpn_underlying_adapter_type_by_name_[network_info.interface_name] =
|
||||
AdapterTypeFromNetworkType(network_info.underlying_type_for_vpn,
|
||||
surface_cellular_types_);
|
||||
}
|
||||
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;
|
||||
}
|
||||
network_handle_by_if_name_[network_info.interface_name] = network_info.handle;
|
||||
RTC_CHECK(network_info_by_handle_.size() >=
|
||||
network_handle_by_if_name_.size());
|
||||
InvokeNetworksChangedCallback();
|
||||
}
|
||||
|
||||
@ -451,12 +458,18 @@ absl::optional<NetworkHandle>
|
||||
AndroidNetworkMonitor::FindNetworkHandleFromIfname(
|
||||
absl::string_view if_name) const {
|
||||
RTC_DCHECK_RUN_ON(network_thread_);
|
||||
|
||||
auto iter = network_handle_by_if_name_.find(if_name);
|
||||
if (iter != network_handle_by_if_name_.end()) {
|
||||
return iter->second;
|
||||
}
|
||||
|
||||
if (bind_using_ifname_) {
|
||||
for (auto const& iter : network_info_by_handle_) {
|
||||
for (auto const& iter : network_handle_by_if_name_) {
|
||||
// Use substring match so that e.g if_name="v4-wlan0" is matched
|
||||
// agains iter="wlan0"
|
||||
if (if_name.find(iter.second.interface_name) != absl::string_view::npos) {
|
||||
return absl::make_optional(iter.first);
|
||||
if (if_name.find(iter.first) != absl::string_view::npos) {
|
||||
return absl::make_optional(iter.second);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -468,12 +481,57 @@ void AndroidNetworkMonitor::OnNetworkDisconnected_n(NetworkHandle handle) {
|
||||
RTC_DCHECK_RUN_ON(network_thread_);
|
||||
RTC_LOG(LS_INFO) << "Network disconnected for handle " << handle;
|
||||
auto iter = network_info_by_handle_.find(handle);
|
||||
if (iter != network_info_by_handle_.end()) {
|
||||
for (const rtc::IPAddress& address : iter->second.ip_addresses) {
|
||||
network_handle_by_address_.erase(address);
|
||||
}
|
||||
network_info_by_handle_.erase(iter);
|
||||
if (iter == network_info_by_handle_.end()) {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const rtc::IPAddress& address : iter->second.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.
|
||||
//
|
||||
// This is handled the following way,
|
||||
// 1) OnNetworkConnected_n overwrites any previous "owner" of an interface
|
||||
// name ("owner" == entry in network_handle_by_if_name_).
|
||||
// 2) OnNetworkDisconnected_n, we scan and see if there are any remaining
|
||||
// connected network with the interface name, and set it as owner.
|
||||
//
|
||||
// This means that network_info_by_handle can have more entries than
|
||||
// network_handle_by_if_name_.
|
||||
|
||||
// Check if we are registered as "owner" of if_name.
|
||||
const auto& if_name = iter->second.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) {
|
||||
// We are owner...
|
||||
// Check if there is someone else we can set as owner.
|
||||
bool found = false;
|
||||
for (const auto& info : network_info_by_handle_) {
|
||||
if (info.first == handle) {
|
||||
continue;
|
||||
}
|
||||
if (info.second.interface_name == if_name) {
|
||||
found = true;
|
||||
network_handle_by_if_name_[if_name] = info.first;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!found) {
|
||||
// No new owner...
|
||||
network_handle_by_if_name_.erase(iter2);
|
||||
}
|
||||
} else {
|
||||
// We are not owner...don't do anything.
|
||||
#if RTC_DCHECK_IS_ON
|
||||
auto owner_handle = FindNetworkHandleFromIfname(if_name);
|
||||
RTC_DCHECK(owner_handle && *owner_handle != handle);
|
||||
#endif
|
||||
}
|
||||
|
||||
network_info_by_handle_.erase(iter);
|
||||
}
|
||||
|
||||
void AndroidNetworkMonitor::OnNetworkPreference_n(
|
||||
@ -491,8 +549,16 @@ void AndroidNetworkMonitor::OnNetworkPreference_n(
|
||||
void AndroidNetworkMonitor::SetNetworkInfos(
|
||||
const std::vector<NetworkInformation>& network_infos) {
|
||||
RTC_DCHECK_RUN_ON(network_thread_);
|
||||
network_handle_by_address_.clear();
|
||||
network_info_by_handle_.clear();
|
||||
|
||||
// We expect this method to be called once directly after startMonitoring.
|
||||
// All the caches should be empty.
|
||||
RTC_DCHECK(network_handle_by_if_name_.empty());
|
||||
RTC_DCHECK(network_handle_by_address_.empty());
|
||||
RTC_DCHECK(network_info_by_handle_.empty());
|
||||
RTC_DCHECK(network_preference_by_adapter_type_.empty());
|
||||
|
||||
// ...but reset just in case.
|
||||
reset();
|
||||
RTC_LOG(LS_INFO) << "Android network monitor found " << network_infos.size()
|
||||
<< " networks";
|
||||
for (const NetworkInformation& network : network_infos) {
|
||||
@ -500,68 +566,43 @@ void AndroidNetworkMonitor::SetNetworkInfos(
|
||||
}
|
||||
}
|
||||
|
||||
rtc::AdapterType AndroidNetworkMonitor::GetAdapterType(
|
||||
absl::string_view if_name) {
|
||||
rtc::NetworkMonitorInterface::InterfaceInfo
|
||||
AndroidNetworkMonitor::GetInterfaceInfo(absl::string_view if_name) {
|
||||
RTC_DCHECK_RUN_ON(network_thread_);
|
||||
auto iter = adapter_type_by_name_.find(if_name);
|
||||
rtc::AdapterType type = (iter == adapter_type_by_name_.end())
|
||||
? rtc::ADAPTER_TYPE_UNKNOWN
|
||||
: iter->second;
|
||||
|
||||
if (type == rtc::ADAPTER_TYPE_UNKNOWN && bind_using_ifname_) {
|
||||
for (auto const& iter : adapter_type_by_name_) {
|
||||
// Use substring match so that e.g if_name="v4-wlan0" is matched
|
||||
// against iter="wlan0"
|
||||
if (if_name.find(iter.first) != absl::string_view::npos) {
|
||||
type = iter.second;
|
||||
break;
|
||||
}
|
||||
}
|
||||
auto handle = FindNetworkHandleFromIfname(if_name);
|
||||
if (!handle) {
|
||||
return {
|
||||
.adapter_type = rtc::ADAPTER_TYPE_UNKNOWN,
|
||||
.available = (disable_is_adapter_available_ ? true : false),
|
||||
};
|
||||
}
|
||||
auto iter = network_info_by_handle_.find(*handle);
|
||||
RTC_DCHECK(iter != network_info_by_handle_.end());
|
||||
if (iter == network_info_by_handle_.end()) {
|
||||
return {
|
||||
.adapter_type = rtc::ADAPTER_TYPE_UNKNOWN,
|
||||
.available = (disable_is_adapter_available_ ? true : false),
|
||||
};
|
||||
}
|
||||
|
||||
if (type == rtc::ADAPTER_TYPE_UNKNOWN) {
|
||||
RTC_LOG(LS_WARNING) << "Get an unknown type for the interface " << if_name;
|
||||
}
|
||||
return type;
|
||||
}
|
||||
|
||||
rtc::AdapterType AndroidNetworkMonitor::GetVpnUnderlyingAdapterType(
|
||||
absl::string_view if_name) {
|
||||
RTC_DCHECK_RUN_ON(network_thread_);
|
||||
auto iter = vpn_underlying_adapter_type_by_name_.find(if_name);
|
||||
rtc::AdapterType type = (iter == vpn_underlying_adapter_type_by_name_.end())
|
||||
? rtc::ADAPTER_TYPE_UNKNOWN
|
||||
: iter->second;
|
||||
if (type == rtc::ADAPTER_TYPE_UNKNOWN && bind_using_ifname_) {
|
||||
// Use partial match so that e.g if_name="v4-wlan0" is matched
|
||||
// agains iter.first="wlan0"
|
||||
for (auto const& iter : vpn_underlying_adapter_type_by_name_) {
|
||||
if (if_name.find(iter.first) != absl::string_view::npos) {
|
||||
type = iter.second;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return type;
|
||||
auto type =
|
||||
AdapterTypeFromNetworkType(iter->second.type, surface_cellular_types_);
|
||||
auto vpn_type =
|
||||
(type == rtc::ADAPTER_TYPE_VPN)
|
||||
? AdapterTypeFromNetworkType(iter->second.underlying_type_for_vpn,
|
||||
surface_cellular_types_)
|
||||
: rtc::ADAPTER_TYPE_UNKNOWN;
|
||||
return {
|
||||
.adapter_type = type,
|
||||
.underlying_type_for_vpn = vpn_type,
|
||||
.network_preference = GetNetworkPreference(type),
|
||||
.available = true,
|
||||
};
|
||||
}
|
||||
|
||||
rtc::NetworkPreference AndroidNetworkMonitor::GetNetworkPreference(
|
||||
absl::string_view if_name) {
|
||||
rtc::AdapterType adapter_type) const {
|
||||
RTC_DCHECK_RUN_ON(network_thread_);
|
||||
auto iter = adapter_type_by_name_.find(if_name);
|
||||
if (iter == adapter_type_by_name_.end()) {
|
||||
return rtc::NetworkPreference::NEUTRAL;
|
||||
}
|
||||
|
||||
rtc::AdapterType adapter_type = iter->second;
|
||||
if (adapter_type == rtc::ADAPTER_TYPE_VPN) {
|
||||
auto iter2 = vpn_underlying_adapter_type_by_name_.find(if_name);
|
||||
if (iter2 != vpn_underlying_adapter_type_by_name_.end()) {
|
||||
adapter_type = iter2->second;
|
||||
}
|
||||
}
|
||||
|
||||
auto preference_iter = network_preference_by_adapter_type_.find(adapter_type);
|
||||
if (preference_iter == network_preference_by_adapter_type_.end()) {
|
||||
return rtc::NetworkPreference::NEUTRAL;
|
||||
@ -570,32 +611,6 @@ rtc::NetworkPreference AndroidNetworkMonitor::GetNetworkPreference(
|
||||
return preference_iter->second;
|
||||
}
|
||||
|
||||
// Check if adapter is avaiable, and only return true for the interface
|
||||
// that has been discovered by NetworkMonitorAutoDetect.java.
|
||||
bool AndroidNetworkMonitor::IsAdapterAvailable(absl::string_view if_name) {
|
||||
RTC_DCHECK_RUN_ON(network_thread_);
|
||||
if (disable_is_adapter_available_) {
|
||||
return true;
|
||||
}
|
||||
if (if_name == "lo") {
|
||||
// localhost (if_name == lo) is used by unit tests.
|
||||
return true;
|
||||
}
|
||||
bool val = adapter_type_by_name_.find(if_name) != adapter_type_by_name_.end();
|
||||
if (!val && bind_using_ifname_) {
|
||||
for (auto const& iter : network_info_by_handle_) {
|
||||
// Use substring match so that e.g if_name="v4-wlan0" is matched
|
||||
// against iter.first="wlan0"
|
||||
if (if_name.find(iter.second.interface_name) != absl::string_view::npos) {
|
||||
val = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return val;
|
||||
}
|
||||
|
||||
AndroidNetworkMonitorFactory::AndroidNetworkMonitorFactory()
|
||||
: j_application_context_(nullptr) {}
|
||||
|
||||
|
||||
@ -29,6 +29,10 @@
|
||||
#include "sdk/android/src/jni/jni_helpers.h"
|
||||
|
||||
namespace webrtc {
|
||||
namespace test {
|
||||
class AndroidNetworkMonitorTest;
|
||||
} // namespace test
|
||||
|
||||
namespace jni {
|
||||
|
||||
typedef int64_t NetworkHandle;
|
||||
@ -88,12 +92,8 @@ class AndroidNetworkMonitor : public rtc::NetworkMonitorInterface {
|
||||
int socket_fd,
|
||||
const rtc::IPAddress& address,
|
||||
absl::string_view if_name) override;
|
||||
rtc::AdapterType GetAdapterType(absl::string_view if_name) override;
|
||||
rtc::AdapterType GetVpnUnderlyingAdapterType(
|
||||
absl::string_view if_name) override;
|
||||
rtc::NetworkPreference GetNetworkPreference(
|
||||
absl::string_view if_name) override;
|
||||
bool IsAdapterAvailable(absl::string_view if_name) override;
|
||||
|
||||
InterfaceInfo GetInterfaceInfo(absl::string_view if_name) override;
|
||||
|
||||
// Always expected to be called on the network thread.
|
||||
void SetNetworkInfos(const std::vector<NetworkInformation>& network_infos);
|
||||
@ -120,11 +120,13 @@ class AndroidNetworkMonitor : public rtc::NetworkMonitorInterface {
|
||||
absl::string_view ifname) const;
|
||||
|
||||
private:
|
||||
void reset();
|
||||
void OnNetworkConnected_n(const NetworkInformation& network_info);
|
||||
void OnNetworkDisconnected_n(NetworkHandle network_handle);
|
||||
void OnNetworkPreference_n(NetworkType type,
|
||||
rtc::NetworkPreference preference);
|
||||
|
||||
rtc::NetworkPreference GetNetworkPreference(rtc::AdapterType) const;
|
||||
absl::optional<NetworkHandle> FindNetworkHandleFromIfname(
|
||||
absl::string_view ifname) const;
|
||||
|
||||
@ -133,10 +135,8 @@ class AndroidNetworkMonitor : public rtc::NetworkMonitorInterface {
|
||||
ScopedJavaGlobalRef<jobject> j_network_monitor_;
|
||||
rtc::Thread* const network_thread_;
|
||||
bool started_ RTC_GUARDED_BY(network_thread_) = false;
|
||||
std::map<std::string, rtc::AdapterType, rtc::AbslStringViewCmp>
|
||||
adapter_type_by_name_ RTC_GUARDED_BY(network_thread_);
|
||||
std::map<std::string, rtc::AdapterType, rtc::AbslStringViewCmp>
|
||||
vpn_underlying_adapter_type_by_name_ RTC_GUARDED_BY(network_thread_);
|
||||
std::map<std::string, NetworkHandle, rtc::AbslStringViewCmp>
|
||||
network_handle_by_if_name_ RTC_GUARDED_BY(network_thread_);
|
||||
std::map<rtc::IPAddress, NetworkHandle> network_handle_by_address_
|
||||
RTC_GUARDED_BY(network_thread_);
|
||||
std::map<NetworkHandle, NetworkInformation> network_info_by_handle_
|
||||
@ -163,6 +163,8 @@ class AndroidNetworkMonitor : public rtc::NetworkMonitorInterface {
|
||||
RTC_PT_GUARDED_BY(network_thread_) = nullptr;
|
||||
|
||||
const FieldTrialsView& field_trials_;
|
||||
|
||||
friend class webrtc::test::AndroidNetworkMonitorTest;
|
||||
};
|
||||
|
||||
class AndroidNetworkMonitorFactory : public rtc::NetworkMonitorFactory {
|
||||
|
||||
Reference in New Issue
Block a user