Merge branch 'netsh-timeout'

5 files changed, 66 insertions, 27 deletions

diff --git a/windows/windns/src/windns/netconfighelpers.cpp b/windows/windns/src/windns/netconfighelpers.cpp
index 65e38b06c3..7156038b4e 100644
--- a/windows/windns/src/windns/netconfighelpers.cpp
+++ b/windows/windns/src/windns/netconfighelpers.cpp
@@ -42,7 +42,7 @@ void SetDnsServers(uint32_t interfaceIndex, const std::vector<std::wstring> &ser
 	}
 }
 
-void RevertDnsServers(const InterfaceConfig &config)
+void RevertDnsServers(const InterfaceConfig &config, uint32_t timeout)
 {
 	XTRACE("Reverting DNS configuration for interface with index=", config.interfaceIndex());
 
@@ -50,15 +50,15 @@ void RevertDnsServers(const InterfaceConfig &config)
 
 	if (config.dhcp() || nullptr == servers || 0 == servers->size())
 	{
-		NetSh::SetIpv4Dhcp(config.interfaceIndex());
+		NetSh::SetIpv4Dhcp(config.interfaceIndex(), timeout);
 		return;
 	}
 
-	NetSh::SetIpv4PrimaryDns(config.interfaceIndex(), (*servers)[0]);
+	NetSh::SetIpv4PrimaryDns(config.interfaceIndex(), (*servers)[0], timeout);
 
 	if (servers->size() > 1)
 	{
-		NetSh::SetIpv4SecondaryDns(config.interfaceIndex(), (*servers)[1]);
+		NetSh::SetIpv4SecondaryDns(config.interfaceIndex(), (*servers)[1], timeout);
 	}
 }
 
diff --git a/windows/windns/src/windns/netconfighelpers.h b/windows/windns/src/windns/netconfighelpers.h
index fbfef34035..33b6c64cc2 100644
--- a/windows/windns/src/windns/netconfighelpers.h
+++ b/windows/windns/src/windns/netconfighelpers.h
@@ -19,6 +19,6 @@ uint32_t GetInterfaceIndex(CComPtr<IWbemClassObject> instance);
 
 void SetDnsServers(uint32_t interfaceIndex, const std::vector<std::wstring> &servers);
 
-void RevertDnsServers(const InterfaceConfig &config);
+void RevertDnsServers(const InterfaceConfig &config, uint32_t timeout = 0);
 
 }
diff --git a/windows/windns/src/windns/netsh.cpp b/windows/windns/src/windns/netsh.cpp
index 7c031db87e..fdd8fbfa03 100644
--- a/windows/windns/src/windns/netsh.cpp
+++ b/windows/windns/src/windns/netsh.cpp
@@ -10,6 +10,8 @@
 namespace
 {
 
+ErrorSinkInfo g_ErrorSink = { nullptr, nullptr };
+
 std::wstring g_NetShPath;
 
 void InitializePath()
@@ -31,6 +33,13 @@ const std::wstring &NetShPath()
 	return g_NetShPath;
 }
 
+void InfoSink(const char *msg)
+{
+	auto infoMsg = std::string("INFO: ").append(msg);
+
+	g_ErrorSink.sink(infoMsg.c_str(), nullptr, 0, g_ErrorSink.context);
+}
+
 std::vector<std::string> BlockToRows(const std::string &textBlock)
 {
 	//
@@ -78,13 +87,16 @@ __declspec(noreturn) void ThrowWithDetails(std::string &&error, common::Applicat
 	throw NetShError(std::move(error), std::move(details));
 }
 
-void ValidateShellOut(common::ApplicationRunner &netsh)
+void ValidateShellOut(common::ApplicationRunner &netsh, uint32_t timeout)
 {
-	static const size_t TIMEOUT_MILLISECONDS = 2000;
+	// Use default timeout of 4 seconds.
+	const uint32_t actualTimeout = (0 == timeout ? 3000 : timeout);
+
+	const auto startTime = GetTickCount64();
 
 	DWORD returnCode;
 
-	if (false == netsh.join(returnCode, TIMEOUT_MILLISECONDS))
+	if (false == netsh.join(returnCode, actualTimeout))
 	{
 		ThrowWithDetails("'netsh' did not complete in a timely manner", netsh);
 	}
@@ -97,12 +109,31 @@ void ValidateShellOut(common::ApplicationRunner &netsh)
 
 		ThrowWithDetails(ss.str(), netsh);
 	}
+
+	const auto elapsed = static_cast<uint32_t>(GetTickCount64() - startTime);
+
+	if (elapsed > (actualTimeout / 2))
+	{
+		std::stringstream ss;
+
+		ss << L"'netsh' completed successfully, albeit a little slowly. It consumed "
+			<< elapsed << " ms of "
+			<< actualTimeout << " ms max permitted execution time";
+
+		InfoSink(ss.str().c_str());
+	}
 }
 
 } // anonymous namespace
 
 //static
-void NetSh::SetIpv4PrimaryDns(uint32_t interfaceIndex, std::wstring server)
+void NetSh::RegisterErrorSink(const ErrorSinkInfo &errorSink)
+{
+	g_ErrorSink = errorSink;
+}
+
+//static
+void NetSh::SetIpv4PrimaryDns(uint32_t interfaceIndex, std::wstring server, uint32_t timeout)
 {
 	//
 	// netsh interface ipv4 set dnsservers name="Ethernet 2" source=static address=8.8.8.8 validate=no
@@ -120,11 +151,11 @@ void NetSh::SetIpv4PrimaryDns(uint32_t interfaceIndex, std::wstring server)
 
 	auto netsh = common::ApplicationRunner::StartWithoutConsole(NetShPath(), ss.str());
 
-	ValidateShellOut(*netsh);
+	ValidateShellOut(*netsh, timeout);
 }
 
 //static
-void NetSh::SetIpv4SecondaryDns(uint32_t interfaceIndex, std::wstring server)
+void NetSh::SetIpv4SecondaryDns(uint32_t interfaceIndex, std::wstring server, uint32_t timeout)
 {
 	//
 	// netsh interface ipv4 add dnsservers name="Ethernet 2" address=8.8.4.4 index=2 validate=no
@@ -142,11 +173,11 @@ void NetSh::SetIpv4SecondaryDns(uint32_t interfaceIndex, std::wstring server)
 
 	auto netsh = common::ApplicationRunner::StartWithoutConsole(NetShPath(), ss.str());
 
-	ValidateShellOut(*netsh);
+	ValidateShellOut(*netsh, timeout);
 }
 
 //static
-void NetSh::SetIpv4Dhcp(uint32_t interfaceIndex)
+void NetSh::SetIpv4Dhcp(uint32_t interfaceIndex, uint32_t timeout)
 {
 	//
 	// netsh interface ipv4 set dnsservers name="Ethernet 2" source=dhcp
@@ -162,11 +193,11 @@ void NetSh::SetIpv4Dhcp(uint32_t interfaceIndex)
 
 	auto netsh = common::ApplicationRunner::StartWithoutConsole(NetShPath(), ss.str());
 
-	ValidateShellOut(*netsh);
+	ValidateShellOut(*netsh, timeout);
 }
 
 //static
-void NetSh::SetIpv6PrimaryDns(uint32_t interfaceIndex, std::wstring server)
+void NetSh::SetIpv6PrimaryDns(uint32_t interfaceIndex, std::wstring server, uint32_t timeout)
 {
 	//
 	// netsh interface ipv6 set dnsservers name="Ethernet 2" source=static address=2001:4860:4860::8888 validate=no
@@ -184,11 +215,11 @@ void NetSh::SetIpv6PrimaryDns(uint32_t interfaceIndex, std::wstring server)
 
 	auto netsh = common::ApplicationRunner::StartWithoutConsole(NetShPath(), ss.str());
 
-	ValidateShellOut(*netsh);
+	ValidateShellOut(*netsh, timeout);
 }
 
 //static
-void NetSh::SetIpv6SecondaryDns(uint32_t interfaceIndex, std::wstring server)
+void NetSh::SetIpv6SecondaryDns(uint32_t interfaceIndex, std::wstring server, uint32_t timeout)
 {
 	//
 	// netsh interface ipv6 add dnsservers name="Ethernet 2" address=2001:4860:4860::8844 index=2 validate=no
@@ -206,11 +237,11 @@ void NetSh::SetIpv6SecondaryDns(uint32_t interfaceIndex, std::wstring server)
 
 	auto netsh = common::ApplicationRunner::StartWithoutConsole(NetShPath(), ss.str());
 
-	ValidateShellOut(*netsh);
+	ValidateShellOut(*netsh, timeout);
 }
 
 //static
-void NetSh::SetIpv6Dhcp(uint32_t interfaceIndex)
+void NetSh::SetIpv6Dhcp(uint32_t interfaceIndex, uint32_t timeout)
 {
 	//
 	// netsh interface ipv6 set dnsservers name="Ethernet 2" source=dhcp
@@ -226,5 +257,5 @@ void NetSh::SetIpv6Dhcp(uint32_t interfaceIndex)
 
 	auto netsh = common::ApplicationRunner::StartWithoutConsole(NetShPath(), ss.str());
 
-	ValidateShellOut(*netsh);
+	ValidateShellOut(*netsh, timeout);
 }
diff --git a/windows/windns/src/windns/netsh.h b/windows/windns/src/windns/netsh.h
index 7aa5b800f0..5ba43fba66 100644
--- a/windows/windns/src/windns/netsh.h
+++ b/windows/windns/src/windns/netsh.h
@@ -1,5 +1,6 @@
 #pragma once
 
+#include "clientsinkinfo.h"
 #include <string>
 #include <cstdint>
 #include <stdexcept>
@@ -8,18 +9,20 @@ class NetSh
 {
 public:
 
-	static void SetIpv4PrimaryDns(uint32_t interfaceIndex, std::wstring server);
+	static void RegisterErrorSink(const ErrorSinkInfo &errorSink);
+
+	static void SetIpv4PrimaryDns(uint32_t interfaceIndex, std::wstring server, uint32_t timeout = 0);
 	
 	//
 	// Caveat: This sets the primary DNS server if there isn't already one.
 	//
-	static void SetIpv4SecondaryDns(uint32_t interfaceIndex, std::wstring server);
+	static void SetIpv4SecondaryDns(uint32_t interfaceIndex, std::wstring server, uint32_t timeout = 0);
 
-	static void SetIpv4Dhcp(uint32_t interfaceIndex);
+	static void SetIpv4Dhcp(uint32_t interfaceIndex, uint32_t timeout = 0);
 
-	static void SetIpv6PrimaryDns(uint32_t interfaceIndex, std::wstring server);
-	static void SetIpv6SecondaryDns(uint32_t interfaceIndex, std::wstring server);
-	static void SetIpv6Dhcp(uint32_t interfaceIndex);
+	static void SetIpv6PrimaryDns(uint32_t interfaceIndex, std::wstring server, uint32_t timeout = 0);
+	static void SetIpv6SecondaryDns(uint32_t interfaceIndex, std::wstring server, uint32_t timeout = 0);
+	static void SetIpv6Dhcp(uint32_t interfaceIndex, uint32_t timeout = 0);
 
 private:
 
diff --git a/windows/windns/src/windns/windns.cpp b/windows/windns/src/windns/windns.cpp
index 662432495a..8bf9a1b649 100644
--- a/windows/windns/src/windns/windns.cpp
+++ b/windows/windns/src/windns/windns.cpp
@@ -5,6 +5,7 @@
 #include "interfaceconfig.h"
 #include "netconfighelpers.h"
 #include "confineoperation.h"
+#include "netsh.h"
 #include "libcommon/serialization/deserializer.h"
 #include <vector>
 #include <string>
@@ -57,6 +58,8 @@ WinDns_Initialize(
 
 	return ConfineOperation("Initialize", ForwardError, []()
 	{
+		NetSh::RegisterErrorSink(ErrorSinkInfo{ g_ErrorSink, g_ErrorContext });
+
 		g_Context = new WinDnsContext;
 	});
 }
@@ -161,13 +164,15 @@ WinDns_Recover(
 	// Try to restore each config and update 'success' if any update fails.
 	//
 
+	static const uint32_t TIMEOUT_10_SECONDS = 10 * 1000;
+
 	bool success = true;
 
 	for (const auto &config : configs)
 	{
 		const auto adapterStatus = ConfineOperation("Restore adapter DNS settings", ForwardError, [&config]()
 		{
-			nchelpers::RevertDnsServers(config);
+			nchelpers::RevertDnsServers(config, TIMEOUT_10_SECONDS);
 		});
 
 		if (false == adapterStatus)