Merge branch 'win-adjust-offline-logic'

1 files changed, 67 insertions, 50 deletions

diff --git a/windows/winnet/src/winnet/netmonitor.cpp b/windows/winnet/src/winnet/netmonitor.cpp
index 99a31f7abb..777925cce3 100644
--- a/windows/winnet/src/winnet/netmonitor.cpp
+++ b/windows/winnet/src/winnet/netmonitor.cpp
@@ -117,7 +117,12 @@ void NetMonitor::AddCacheEntry(Cache &cache, const MIB_IF_ROW2 &iface)
 	{
 		e.luid = iface.InterfaceLuid.Value;
 		e.valid = true;
-		e.connected = (MediaConnectStateConnected == iface.MediaConnectState);
+		e.connected =
+		(
+			NET_IF_ADMIN_STATUS_UP == iface.AdminStatus
+			&& IfOperStatusUp == iface.OperStatus
+			&& MediaConnectStateConnected == iface.MediaConnectState
+		);
 	}
 	else
 	{
@@ -153,7 +158,20 @@ void NetMonitor::updateConnectivity()
 //static
 void __stdcall NetMonitor::Callback(void *context, MIB_IPINTERFACE_ROW *hint, MIB_NOTIFICATION_TYPE updateType)
 {
-	reinterpret_cast<NetMonitor *>(context)->callback(hint, updateType);
+	auto nm = reinterpret_cast<NetMonitor *>(context);
+
+	try
+	{
+		nm->callback(hint, updateType);
+	}
+	catch (const std::exception &err)
+	{
+		nm->m_logSink->error(err.what());
+	}
+	catch (...)
+	{
+		nm->m_logSink->error("Unspecified error in NetMonitor::Callback()");
+	}
 }
 
 void NetMonitor::callback(MIB_IPINTERFACE_ROW *hint, MIB_NOTIFICATION_TYPE updateType)
@@ -167,72 +185,71 @@ void NetMonitor::callback(MIB_IPINTERFACE_ROW *hint, MIB_NOTIFICATION_TYPE updat
 			MIB_IF_ROW2 iface = { 0 };
 			iface.InterfaceLuid = hint->InterfaceLuid;
 
-			if (NO_ERROR != GetIfEntry2(&iface))
+			const auto status = GetIfEntry2(&iface);
+
+			if (NO_ERROR != status)
 			{
-				// Failed to query interface.
-				return;
+				std::stringstream ss;
+
+				ss << "GetIfEntry2() failed for LUID 0x" << std::hex << iface.InterfaceLuid.Value
+					<< " during processing of MibAddInstance, error: 0x" << status;
+
+				throw std::runtime_error(ss.str());
 			}
 
+			//
+			// The reason for removing an existing entry is that enabling
+			// an interface on the adapter might change the overall properties in the
+			// "row" which is merely an abstraction over all interfaces.
+			//
+
+			m_cache.erase(iface.InterfaceLuid.Value);
 			AddCacheEntry(m_cache, iface);
 
 			break;
 		}
 		case MibDeleteInstance:
 		{
-			const auto cacheEntry = m_cache.find(hint->InterfaceLuid.Value);
+			m_cache.erase(hint->InterfaceLuid.Value);
 
-			if (m_cache.end() != cacheEntry)
+			MIB_IF_ROW2 iface = { 0 };
+			iface.InterfaceLuid = hint->InterfaceLuid;
+
+			const auto status = GetIfEntry2(&iface);
+
+			if (NO_ERROR == status)
 			{
-				cacheEntry->second.connected = false;
+				AddCacheEntry(m_cache, iface);
 			}
 
 			break;
 		}
 		case MibParameterNotification:
 		{
-			auto cacheEntry = m_cache.find(hint->InterfaceLuid.Value);
-
-			if (m_cache.end() == cacheEntry)
-			{
-				//
-				// A change occurred on an interface that we're not tracking.
-				// Perhaps the MibAddInstance logic failed for some reason.
-				//
-
-				MIB_IF_ROW2 iface = { 0 };
-				iface.InterfaceLuid = hint->InterfaceLuid;
+			MIB_IF_ROW2 iface = { 0 };
+			iface.InterfaceLuid = hint->InterfaceLuid;
 
-				if (NO_ERROR != GetIfEntry2(&iface))
-				{
-					// Failed to query interface.
-					return;
-				}
+			const auto status = GetIfEntry2(&iface);
 
-				AddCacheEntry(m_cache, iface);
-			}
-			else
+			if (NO_ERROR != status)
 			{
 				//
-				// Abort processing if this is a known interface that we don't care about.
-				//
-				if (false == cacheEntry->second.valid)
-				{
-					return;
-				}
-
-				//
-				// Update cache.
+				// Only update the cache if we can look up the interface details.
+				// This way, if the interface was connected and continues to be so, we don't
+				// mistakenly switch the status to "offline".
 				//
 
-				MIB_IF_ROW2 iface = { 0 };
-				iface.InterfaceLuid = hint->InterfaceLuid;
+				std::stringstream ss;
 
-				const auto status = GetIfEntry2(&iface);
+				ss << "GetIfEntry2() failed for LUID 0x" << std::hex << iface.InterfaceLuid.Value
+					<< " during processing of MibParameterNotification, error: 0x" << status;
 
-				cacheEntry->second.connected =
-					(NO_ERROR == status ? MediaConnectStateConnected == iface.MediaConnectState : false);
+				throw std::runtime_error(ss.str());
 			}
 
+			m_cache.erase(iface.InterfaceLuid.Value);
+			AddCacheEntry(m_cache, iface);
+
 			break;
 		}
 	}
@@ -287,11 +304,6 @@ void NetMonitor::LogOfflineState(std::shared_ptr<common::logging::ILogSink> logS
 	{
 		const auto &iface = table->Table[i];
 
-		std::stringstream ss;
-
-		ss << "Detailed interface logging" << std::endl;
-		ss << "Interface ordinal " << i << std::endl;
-
 		//
 		// Don't flood the log with garbage.
 		//
@@ -305,6 +317,9 @@ void NetMonitor::LogOfflineState(std::shared_ptr<common::logging::ILogSink> logS
 			L"Microsoft Teredo Tunneling Adapter",
 			L"Microsoft IP-HTTPS Platform Adapter",
 			L"Microsoft 6to4 Adapter",
+			L"WAN Miniport",
+			L"WiFi Filter Driver",
+			L"Microsoft Wi-Fi Direct Virtual Adapter",
 		};
 
 		bool blacklisted = false;
@@ -320,19 +335,21 @@ void NetMonitor::LogOfflineState(std::shared_ptr<common::logging::ILogSink> logS
 
 		if (blacklisted)
 		{
-			ss << "  filtered out to avoid flooding log";
-			logSink->info(ss.str().c_str());
-
 			continue;
 		}
 
+		std::stringstream ss;
+
+		ss << "Detailed interface logging" << std::endl;
+		ss << "Interface ordinal " << i << std::endl;
+
 		{
 			const auto s = std::wstring(L"  Alias: ").append(iface.Alias);
 			ss << common::string::ToAnsi(s) << std::endl;
 		}
 
 		{
-			const auto s = std::wstring(L"  \"Description\": ").append(iface.Description);
+			const auto s = std::wstring(L"  Description: ").append(iface.Description);
 			ss << common::string::ToAnsi(s) << std::endl;
 		}