From 39c7c29e69e885e5996fddaef473d0bcd1ced0e0 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Thu, 27 Nov 2025 14:30:54 -0500 Subject: [PATCH] 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 | 1 + src/servers/net/AutoconfigClient.cpp | 2 +- src/servers/net/AutoconfigClient.h | 2 +- src/servers/net/AutoconfigLooper.cpp | 18 +++++-- src/servers/net/AutoconfigLooper.h | 1 + src/servers/net/DHCPClient.cpp | 74 +++++++++++++++++++++------- src/servers/net/DHCPClient.h | 6 ++- src/servers/net/NetServer.cpp | 11 ++--- 8 files changed, 84 insertions(+), 31 deletions(-) diff --git a/headers/private/net/NetServer.h b/headers/private/net/NetServer.h index e0901e3782..60df9e8af9 100644 --- a/headers/private/net/NetServer.h +++ b/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' diff --git a/src/servers/net/AutoconfigClient.cpp b/src/servers/net/AutoconfigClient.cpp index b7e424f2af..42eb041a7f 100644 --- a/src/servers/net/AutoconfigClient.cpp +++ b/src/servers/net/AutoconfigClient.cpp @@ -25,7 +25,7 @@ AutoconfigClient::~AutoconfigClient() status_t -AutoconfigClient::Initialize() +AutoconfigClient::Start() { return B_NOT_SUPPORTED; } diff --git a/src/servers/net/AutoconfigClient.h b/src/servers/net/AutoconfigClient.h index 724bc0a1aa..abfdf528e4 100644 --- a/src/servers/net/AutoconfigClient.h +++ b/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(); } diff --git a/src/servers/net/AutoconfigLooper.cpp b/src/servers/net/AutoconfigLooper.cpp index a68cbf03d0..42bae2a7da 100644 --- a/src/servers/net/AutoconfigLooper.cpp +++ b/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; diff --git a/src/servers/net/AutoconfigLooper.h b/src/servers/net/AutoconfigLooper.h index 4ec5a5427f..948b9b4d6f 100644 --- a/src/servers/net/AutoconfigLooper.h +++ b/src/servers/net/AutoconfigLooper.h @@ -29,6 +29,7 @@ public: private: void _RemoveClient(); void _ConfigureIPv4(); + void _ConfigureIPv4Failed(); void _ReadyToRun(); void _NetworkMonitorNotification(BMessage* message); diff --git a/src/servers/net/DHCPClient.cpp b/src/servers/net/DHCPClient.cpp index 32bd3e953d..c1e0e97b28 100644 --- a/src/servers/net/DHCPClient.cpp +++ b/src/servers/net/DHCPClient.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #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: diff --git a/src/servers/net/DHCPClient.h b/src/servers/net/DHCPClient.h index 4383d38a5f..e5351000ce 100644 --- a/src/servers/net/DHCPClient.h +++ b/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; diff --git a/src/servers/net/NetServer.cpp b/src/servers/net/NetServer.cpp index 3de34230da..94a079709a 100644 --- a/src/servers/net/NetServer.cpp +++ b/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; }