net_server: Run DHCP negotiation on a separate thread to fix hangs.

Previously we ran DHCP negotiation on the looper thread. This meant
that we just blocked it for long periods of time, which stopped
configuration settings changes from working properly, and causing
the Network preferences panel to hang when net_server failed to
respond to its messages.

Now, we spawn a separate thread to do the real work on, and unlock
the looper around the recvfrom() with long timeouts. There's now a
kMsgAutoConfigureFailed that is sent when auto-configuration fails,
so that the AutoconfigLooper can fallback as before. Additionally,
the main NetServer class quits loopers for devices on any configuration
change, rather than deferring it unecessarily (and possibly wrongly, in
some cases.)

Fixes #18037. Probably fixes #17300 and may help with others.

headers/private/net/NetServer.h

@@ -12,6 +12,7 @@
 #define kNetServerSignature		"application/x-vnd.haiku-net_server"
 
 #define kMsgConfigureInterface		'COif'
+#define kMsgAutoConfigureFailed		'COaf'
 #define kMsgConfigureResolver		'COrs'
 #define kMsgCountPersistentNetworks	'CPnw'
 #define kMsgGetPersistentNetwork	'GPnw'

src/servers/net/AutoconfigClient.cpp

@@ -25,7 +25,7 @@ AutoconfigClient::~AutoconfigClient()
 
 
 status_t
-AutoconfigClient::Initialize()
+AutoconfigClient::Start()
 {
 	return B_NOT_SUPPORTED;
 }

src/servers/net/AutoconfigClient.h

@@ -20,7 +20,7 @@ public:
 									BMessenger target, const char* device);
 	virtual						~AutoconfigClient();
 
-	virtual	status_t			Initialize();
+	virtual	status_t			Start();
 
 			const BMessenger&	Target() const { return fTarget; }
 			const char*			Device() const { return fDevice.String(); }

src/servers/net/AutoconfigLooper.cpp

@@ -43,6 +43,7 @@ AutoconfigLooper::AutoconfigLooper(BMessenger target, const char* device)
 
 AutoconfigLooper::~AutoconfigLooper()
 {
+	_RemoveClient();
 }
 
 
@@ -52,7 +53,6 @@ AutoconfigLooper::_RemoveClient()
 	if (fCurrentClient == NULL)
 		return;
 
-	RemoveHandler(fCurrentClient);
 	delete fCurrentClient;
 	fCurrentClient = NULL;
 }
@@ -74,9 +74,16 @@ AutoconfigLooper::_ConfigureIPv4()
 	int32 flags = interface.Flags() & ~IFF_AUTO_CONFIGURED;
 	interface.SetFlags(flags | IFF_CONFIGURING);
 
-	if (fCurrentClient->Initialize() == B_OK)
+	if (fCurrentClient->Start() == B_OK)
 		return;
 
+	_ConfigureIPv4Failed();
+}
+
+
+void
+AutoconfigLooper::_ConfigureIPv4Failed()
+{
 	_RemoveClient();
 
 	puts("DHCP failed miserably!");
@@ -85,6 +92,7 @@ AutoconfigLooper::_ConfigureIPv4()
 	// TODO: have a look at zeroconf
 	// TODO: this could also be done add-on based
 
+	BNetworkInterface interface(fDevice.String());
 	if ((interface.Flags() & IFF_CONFIGURING) == 0) {
 		// Someone else configured the interface in the mean time
 		return;
@@ -176,7 +184,7 @@ AutoconfigLooper::_NetworkMonitorNotification(BMessage* message)
 				// Reconfigure the interface when we have a link again
 				_ConfigureIPv4();
 				//_ConfigureIPv6();	// TODO: router advertisement and dhcpv6
-			} else if ((fLastMediaStatus & IFM_ACTIVE) != 0 && (media & IFM_ACTIVE) == 0) {
+			} else if ((media & IFM_ACTIVE) == 0) {
 				_RemoveClient();
 			}
 
@@ -216,6 +224,10 @@ AutoconfigLooper::MessageReceived(BMessage* message)
 			_ReadyToRun();
 			break;
 
+		case kMsgAutoConfigureFailed:
+			_ConfigureIPv4Failed();
+			break;
+
 		case B_NETWORK_MONITOR:
 			_NetworkMonitorNotification(message);
 			break;

src/servers/net/AutoconfigLooper.h

@@ -29,6 +29,7 @@ public:
 private:
 			void				_RemoveClient();
 			void				_ConfigureIPv4();
+			void				_ConfigureIPv4Failed();
 			void				_ReadyToRun();
 			void				_NetworkMonitorNotification(BMessage* message);
 

src/servers/net/DHCPClient.cpp

@@ -30,6 +30,7 @@
 #include <Debug.h>
 #include <Message.h>
 #include <MessageRunner.h>
+#include <Looper.h>
 
 #include "NetServer.h"
 
@@ -482,6 +483,7 @@ DHCPClient::DHCPClient(BMessenger target, const char* device)
 	fConfiguration(kMsgConfigureInterface),
 	fResolverConfiguration(kMsgConfigureResolver),
 	fRunner(NULL),
+	fNegotiateThread(-1),
 	fAssignedAddress(0),
 	fServer(AF_INET, NULL, DHCP_SERVER_PORT, B_UNCONFIGURED_ADDRESS_FAMILIES),
 	fStartTime(0),
@@ -518,18 +520,27 @@ DHCPClient::DHCPClient(BMessenger target, const char* device)
 			}
 		}
 	}
-
-	openlog_thread("DHCP", 0, LOG_DAEMON);
 }
 
 
 DHCPClient::~DHCPClient()
 {
-	if (fStatus != B_OK)
-		return;
+	thread_id thread = fNegotiateThread;
+	if (thread != -1) {
+		fNegotiateThread = -1;
+		suspend_thread(thread);
+		resume_thread(thread);
+
+		UnlockLooper();
+		wait_for_thread(thread, NULL);
+		LockLooper();
+	}
 
 	delete fRunner;
 
+	if (fStatus != B_OK)
+		return;
+
 	int socket = ::socket(AF_INET, SOCK_DGRAM, 0);
 	if (socket < 0)
 		return;
@@ -541,23 +552,42 @@ DHCPClient::~DHCPClient()
 
 	_SendMessage(socket, release, fServer);
 	close(socket);
+}
+
+
+status_t
+DHCPClient::Start()
+{
+	if (fNegotiateThread != -1)
+		return EINPROGRESS;
+
+	fNegotiateThread = spawn_thread(_NegotiatorThread, "DHCP negotiator",
+		B_NORMAL_PRIORITY, this);
+	return resume_thread(fNegotiateThread);
+}
+
+
+status_t
+DHCPClient::_NegotiatorThread(void* data)
+{
+	openlog_thread("DHCP", 0, LOG_DAEMON);
+
+	DHCPClient* client = (DHCPClient*)data;
+	client->LockLooper();
+	client->fStatus = client->_Negotiate();
+	syslog(LOG_DEBUG, "%s: DHCP status = %s\n", client->Device(), strerror(client->fStatus));
+	client->fNegotiateThread = -1;
+	client->UnlockLooper();
 
 	closelog_thread();
+	return B_OK;
 }
 
 
 status_t
-DHCPClient::Initialize()
-{
-	fStatus = _Negotiate(fAssignedAddress == 0 ? INIT : INIT_REBOOT);
-	syslog(LOG_DEBUG, "%s: DHCP status = %s\n", Device(), strerror(fStatus));
-	return fStatus;
-}
-
-
-status_t
-DHCPClient::_Negotiate(dhcp_state state)
+DHCPClient::_Negotiate()
 {
+	dhcp_state state = _CurrentState();
 	if (state == BOUND)
 		return B_OK;
 
@@ -595,7 +625,7 @@ DHCPClient::_Negotiate(dhcp_state state)
 	bigtime_t previousLeaseTime = fLeaseTime;
 
 	status_t status = B_OK;
-	while (state != BOUND) {
+	while (state != BOUND && fNegotiateThread != -1) {
 		status = _StateTransition(socket, state);
 		if (status != B_OK && (state == SELECTING || state == REBOOTING))
 			break;
@@ -618,8 +648,12 @@ DHCPClient::_Negotiate(dhcp_state state)
 	_RestartLease(fRenewalTime);
 
 	fStatus = status;
-	if (status)
+	if (status != B_OK) {
+		// If we were asked to quit, don't inform the looper that we failed.
+		if (fNegotiateThread != -1)
+			Looper()->PostMessage(kMsgAutoConfigureFailed, Looper(), this);
 		return status;
+	}
 
 	// configure interface
 	BMessage reply;
@@ -737,8 +771,12 @@ DHCPClient::_StateTransition(int socket, dhcp_state& state)
 		char buffer[2048];
 		struct sockaddr_in from;
 		socklen_t fromLength = sizeof(from);
+
+		UnlockLooper();
 		ssize_t bytesReceived = recvfrom(socket, buffer, sizeof(buffer),
 			0, (struct sockaddr*)&from, &fromLength);
+		LockLooper();
+
 		if (bytesReceived < 0 && errno == B_TIMED_OUT) {
 			// depending on the state, we'll just try again
 			if (!_TimeoutShift(socket, state, timeout))
@@ -1032,7 +1070,7 @@ DHCPClient::_CurrentState() const
 	bigtime_t now = system_time();
 
 	if (now > fLeaseTime || fStatus != B_OK)
-		return INIT;
+		return fAssignedAddress == 0 ? INIT : INIT_REBOOT;
 	if (now >= fRebindingTime)
 		return REBINDING;
 	if (now >= fRenewalTime)
@@ -1046,7 +1084,7 @@ DHCPClient::MessageReceived(BMessage* message)
 {
 	switch (message->what) {
 		case kMsgLeaseTime:
-			_Negotiate(_CurrentState());
+			Start();
 			break;
 
 		default:

src/servers/net/DHCPClient.h

@@ -39,12 +39,13 @@ public:
 									const char* device);
 	virtual						~DHCPClient();
 
-	virtual	status_t			Initialize();
+	virtual	status_t			Start();
 
 	virtual	void				MessageReceived(BMessage* message);
 
 private:
-			status_t			_Negotiate(dhcp_state state);
+	static	status_t			_NegotiatorThread(void* data);
+			status_t			_Negotiate();
 			status_t			_GotMessage(dhcp_state& state,
 									dhcp_message* message);
 			status_t			_StateTransition(int socket, dhcp_state& state);
@@ -67,6 +68,7 @@ private:
 			BMessage			fConfiguration;
 			BMessage			fResolverConfiguration;
 			BMessageRunner*		fRunner;
+			thread_id			fNegotiateThread;
 			uint8				fMAC[6];
 			BString				fHostName;
 			uint32				fTransactionID;

src/servers/net/NetServer.cpp

@@ -426,10 +426,12 @@ NetServer::_ConfigureInterface(BMessage& message)
 	if (message.FindInt32("flags", &flags) != B_OK)
 		flags = IFF_UP;
 
-	bool autoConfigured;
+	bool autoConfigured = false;
 	if (message.FindBool("auto_configured", &autoConfigured) == B_OK
 			&& autoConfigured) {
 		flags |= IFF_AUTO_CONFIGURED;
+	} else {
+		_QuitLooperForDevice(name);
 	}
 
 	int32 mtu;
@@ -465,10 +467,8 @@ NetServer::_ConfigureInterface(BMessage& message)
 			&addressMessage) == B_OK; index++) {
 		BNetworkInterfaceAddressSettings addressSettings(addressMessage);
 
-		if (addressSettings.IsAutoConfigure()) {
-			_QuitLooperForDevice(name);
+		if (addressSettings.IsAutoConfigure())
 			startAutoConfig = true;
-		}
 
 		// set address/mask/broadcast/peer
 
@@ -570,8 +570,7 @@ NetServer::_ConfigureInterface(BMessage& message)
 		looper->Run();
 
 		fDeviceMap[name] = looper;
-	} else if (!autoConfigured)
-		_QuitLooperForDevice(name);
+	}
 
 	return B_OK;
 }