Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(windows): fix race condition in WiFi connection details feature #568

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion example/ConnectionInfoSubscription.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export default class ConnectionInfoSubscription extends React.Component<
return (
<View>
<Text style={{color: 'black'}}>
{JSON.stringify(this.state.connectionInfoHistory)}
{JSON.stringify(this.state.connectionInfoHistory, null, 4)}
jfkm69 marked this conversation as resolved.
Show resolved Hide resolved
</Text>
</View>
);
Expand Down
64 changes: 43 additions & 21 deletions windows/RNCNetInfoCPP/RNCNetInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,29 +105,43 @@ namespace winrt::ReactNativeNetInfo::implementation {
co_return nullptr;
}

void RNCNetInfo::Initialize(winrt::Microsoft::ReactNative::ReactContext const& /*reactContext*/) noexcept {
IAsyncAction RNCNetInfo::ChainGetNetworkStatus(IAsyncAction previousRequest, std::future<NetInfoState> currentRequest, std::function<void(NetInfoState)> onComplete) {
jfkm69 marked this conversation as resolved.
Show resolved Hide resolved
auto state = co_await currentRequest;
if (previousRequest) {
co_await previousRequest;
}
onComplete(state);
}

void RNCNetInfo::Initialize(winrt::Microsoft::ReactNative::ReactContext const& /*reactContext*/) noexcept {
// NetworkStatusChanged callback is captured by value on purpose. The event handler is called asynchronously and thus can fire even
// after we've already revoked it in our destructor during module teardown. In such a case, a reference
// to "this" or "this->NetworkStatusChanged" would be invalid.
m_networkStatusChangedRevoker = NetworkInformation::NetworkStatusChanged(winrt::auto_revoke, [callback = NetworkStatusChanged](const winrt::IInspectable& /*sender*/) -> winrt::fire_and_forget {
m_networkStatusChangedRevoker = NetworkInformation::NetworkStatusChanged(winrt::auto_revoke, [callback = NetworkStatusChanged, previousRequest = IAsyncAction(nullptr), mutex = std::make_unique<std::mutex>()](const winrt::IInspectable& /*sender*/) mutable {
try {
// Copy lambda capture into a local so it still exists after the co_await.
auto localCallback = callback;
localCallback(co_await GetNetworkStatus());
// Kick off building a status object. Most of this will run synchronously, but getting extra WiFi details will run async at the end.
auto currentRequest = GetNetworkStatus();
// To guarantee ordering of events sent to JS, wait for any previous NetworkStatusChanged events to be processed.
// We atomically swap our latest request into place so that the next downstream NetworkStatusChanged can wait on us.
// Note that we're NOT blocking inside the lock. Either ChainGetNetworkStatus completes synchronously or it hits real async work and yields. But we don't
// wait on the response, we just store the IAsyncAction object.
{
std::scoped_lock lock(*mutex);
jfkm69 marked this conversation as resolved.
Show resolved Hide resolved
IAsyncAction previousRequest = ChainGetNetworkStatus(previousRequest, std::move(currentRequest), callback);
jfkm69 marked this conversation as resolved.
Show resolved Hide resolved
}
}
catch (...) {}
});
});
}

winrt::fire_and_forget RNCNetInfo::getCurrentState(std::string requestedInterface, winrt::Microsoft::ReactNative::ReactPromise<NetInfoState> promise) noexcept {
// Jump to background to avoid blocking the JS thread while we gather the requested data
co_await winrt::resume_background();

promise.Resolve(co_await GetNetworkStatus());
promise.Resolve(co_await GetNetworkStatus(requestedInterface));
}

/*static*/ std::future<NetInfoState> RNCNetInfo::GetNetworkStatus() {
/*static*/ std::future<NetInfoState> RNCNetInfo::GetNetworkStatus(std::string const& requestedInterface) {
NetInfoState state{};

// https://docs.microsoft.com/en-us/uwp/api/windows.networking.connectivity.connectionprofile
Expand All @@ -138,19 +152,17 @@ namespace winrt::ReactNativeNetInfo::implementation {
auto connectivityLevel = profile.GetNetworkConnectivityLevel();
auto signal = profile.GetSignalBars();
auto costType = profile.GetConnectionCost().NetworkCostType();
auto isWifiConnection = profile.IsWlanConnectionProfile() || requestedInterface.find("wifi") != std::string::npos;
auto isCellularConnection = profile.IsWwanConnectionProfile() || requestedInterface.find("cellular") != std::string::npos;
auto isEthernetConnection = networkAdapter || requestedInterface.find("ethernet") != std::string::npos;
auto isConnectionExpensive = costType == NetworkCostType::Fixed || costType == NetworkCostType::Variable;

state.isConnected = connectivityLevel != NetworkConnectivityLevel::None;

if (state.isConnected) {
NetInfoDetails details{};

state.isInternetReachable = connectivityLevel == NetworkConnectivityLevel::InternetAccess;
details.isConnectionExpensive = costType == NetworkCostType::Fixed || costType == NetworkCostType::Variable;
if (signal) {
details.strength = winrt::unbox_value<uint8_t>(signal) * 20; // Signal strength is 0-5 but we want 0-100.
}
if (isWifiConnection) {
NetInfoWifiDetails details{};

if (profile.IsWlanConnectionProfile()) {
auto wlanDetails = profile.WlanConnectionProfileDetails();
auto ssid = wlanDetails.GetConnectedSsid();
jfkm69 marked this conversation as resolved.
Show resolved Hide resolved
auto network = co_await GetWiFiNetwork(networkAdapter, ssid);
Expand All @@ -162,15 +174,26 @@ namespace winrt::ReactNativeNetInfo::implementation {
details.frequency = network.ChannelCenterFrequencyInKilohertz() / 1000; // Convert to Mhz
details.wifiGeneration = GetWifiGeneration(network.PhyKind());
}
if (signal) {
details.strength = winrt::unbox_value<uint8_t>(signal) * 20; // Signal strength is 0-5 but we want 0-100.
jfkm69 marked this conversation as resolved.
Show resolved Hide resolved
}
details.isConnectionExpensive = isConnectionExpensive;

state.details = std::move(details);
}
else if (profile.IsWwanConnectionProfile()) {
else if (isCellularConnection) {
NetInfoCellularDetails details{};

auto wwanDetails = profile.WwanConnectionProfileDetails();
auto dataClass = wwanDetails.GetCurrentDataClass();

state.type = CONNECTION_TYPE_CELLULAR;
details.cellularGeneration = GetCellularGeneration(dataClass);
details.cellularGeneration = GetCellularGeneration(dataClass);
details.isConnectionExpensive = isConnectionExpensive;

state.details = std::move(details);
}
else if (networkAdapter) {
else if (isEthernetConnection) {
// Possible values: https://docs.microsoft.com/en-us/uwp/api/windows.networking.connectivity.networkadapter.ianainterfacetype
if (networkAdapter.IanaInterfaceType() == 6u) {
state.type = CONNECTION_TYPE_ETHERNET;
Expand All @@ -182,8 +205,7 @@ namespace winrt::ReactNativeNetInfo::implementation {
else {
state.type = CONNECTION_TYPE_UNKNOWN;
}

state.details = std::move(details);
state.isInternetReachable = connectivityLevel == NetworkConnectivityLevel::InternetAccess;
}
}
}
Expand Down
9 changes: 8 additions & 1 deletion windows/RNCNetInfoCPP/RNCNetInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,16 @@ namespace winrt::ReactNativeNetInfo::implementation {
struct NetInfoDetails {
REACT_FIELD(isConnectionExpensive);
bool isConnectionExpensive;
};

REACT_STRUCT(NetInfoCellularDetails);
struct NetInfoCellularDetails : NetInfoDetails {
jfkm69 marked this conversation as resolved.
Show resolved Hide resolved
REACT_FIELD(cellularGeneration);
std::optional<std::string> cellularGeneration;
};

REACT_STRUCT(NetInfoWifiDetails);
struct NetInfoWifiDetails : NetInfoDetails {
REACT_FIELD(wifiGeneration);
std::optional<std::string> wifiGeneration;

Expand Down Expand Up @@ -74,7 +80,8 @@ namespace winrt::ReactNativeNetInfo::implementation {
REACT_EVENT(NetworkStatusChanged, L"netInfo.networkStatusDidChange");
std::function<void(NetInfoState)> NetworkStatusChanged;

static std::future<NetInfoState> GetNetworkStatus();
static std::future<NetInfoState> GetNetworkStatus(std::string const& requestedInterface = "");
static IAsyncAction ChainGetNetworkStatus(IAsyncAction previousRequest, std::future<NetInfoState> currentRequest, std::function<void(NetInfoState)> onComplete);
jfkm69 marked this conversation as resolved.
Show resolved Hide resolved

private:
winrt::Windows::Networking::Connectivity::NetworkInformation::NetworkStatusChanged_revoker m_networkStatusChangedRevoker{};
Expand Down
1 change: 1 addition & 0 deletions windows/RNCNetInfoCPP/RNCNetInfoCPP.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
<ImportGroup Label="ExtensionSettings">
</ImportGroup>
<ImportGroup Label="Shared" />
<ImportGroup Label="PropertySheets">
<Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" />
</ImportGroup>
Expand Down