EC: sync clients also fire OnLost on read/write error so amuleweb / amulecmd exit cleanly

got3nks · got3nks · commit 766bd6ffa5d0 · 2026-05-28T17:05:15.000+02:00
In sync mode (use_events=false on CECMuleSocket → amulecmd, amuleweb) there's no pending async_read after the auth handshake, so the EOF that fires HandleRead → PostLostEvent for async clients (amulegui) never gets seen. SendRecvPacket detects the failure on its next ReadSync (or WriteSync, eventually after keepalive teardown) and returns NULL, but nothing notifies the EC layer — amuleweb continues serving HTTP template shells forever with no live amuled data, no error visible to the user, no exit. amulecmd sits at its prompt with a dead socket and every command silently failing. Fix: ReadSync / WriteSync, on a non-zero error_code, dispatch OnLost(0) directly through the LibSocket wrapper (DispatchSyncLost helper) — same dispatch path the asio reactor would take for async clients, just synchronous since the sync-mode caller is already on the main thread. From there: * CECMuleSocket::OnLost(int) forwards through static_cast<CECSocket*> to the EC-layer virtual. * CRemoteConnect::OnLost: if m_notifier is set (amulegui), posts the existing wxEVT_EC_CONNECTION event and the GUI handler flips the UI. If NULL (amuleweb, amulecmd), prints "External Connection lost — exiting" and calls _exit(1) so the supervisor (systemd / shell / docker) decides whether to restart and reconnect. Direct synchronous dispatch (rather than PostLostEvent + wxQueueEvent) because amulecmd's main thread is in fgets reading stdin, not in wxApp's event loop, so queued events would never be processed. amuleweb's wxApp::OnRun is the main loop but it's not running while the HTTP request handler is on the stack — direct dispatch from the same sync-mode call site avoids both reactor and event-loop coordination questions. _exit instead of exit() because we're calling from the main thread holding stacks the asio threads will tear down on shutdown; racing static destructors against them risks the kind of UAF amule-project#748 was about. The OS reaps fds / sockets on process exit anyway. Live-verified on Linux VM: * amulegui: TCP-level FIN observed, UI flips to "Connection failure" within ~1s. * amuleweb: First HTTP request after amuled killed returns with stale template, then amuleweb exits before next request can be served (curl gets connection refused). * amulecmd: First "status" command after amuled killed prints "External Connection lost — exiting" to stderr and exits with code 1. User sees their shell prompt return. Refs amule-project#757.
diff --git a/src/LibSocketAsio.cpp b/src/LibSocketAsio.cpp
@@ -716,6 +716,9 @@ class CAsioSocketImpl : public std::enable_shared_from_this<CAsioSocketImpl>
 		error_code ec;
 		uint32 received = read(*m_socket, buffer(buf, bytesToRead), ec);
 		SetError(ec);
+		if (ec) {
+			DispatchSyncLost();
+		}
 		return received;
 	}
 
@@ -724,9 +727,32 @@ class CAsioSocketImpl : public std::enable_shared_from_this<CAsioSocketImpl>
 		error_code ec;
 		uint32 sent = write(*m_socket, buffer(buf, nbytes), ec);
 		SetError(ec);
+		if (ec) {
+			DispatchSyncLost();
+		}
 		return sent;
 	}
 
+	// Sync clients (amulecmd, amuleweb) don't have an async_read pending
+	// after auth, so the EOF that fires HandleRead → PostLostEvent for
+	// async clients never gets seen. Detection happens here instead, in
+	// ReadSync / WriteSync. PostLostEvent + wxQueueEvent would round-
+	// trip through the wx event loop — which amuleweb has (wxApp::OnRun)
+	// but amulecmd doesn't (its main thread is in fgets reading stdin,
+	// not in wxApp's event loop, so queued events are never processed).
+	// Direct synchronous dispatch through the same wrapper->OnLost(0)
+	// path the async reactor uses covers both: CECMuleSocket::OnLost(int)
+	// forwards to the EC-layer CECSocket::OnLost virtual, CRemoteConnect's
+	// override fires (NULL notifier → _exit fallback) and the headless
+	// EC client exits cleanly instead of serving stale data in limp mode.
+	void DispatchSyncLost()
+	{
+		CLibSocket * wrapper = m_libSocket.load(std::memory_order_acquire);
+		if (wrapper && !m_destroying.load(std::memory_order_acquire) && !m_closed) {
+			wrapper->OnLost(0);
+		}
+	}
+
 	//
 	// Access to even const & wxString is apparently not thread-safe.
 	// Locks are set/removed in wx and reference counts can go astray.
diff --git a/src/libs/ec/cpp/ECMuleSocket.cpp b/src/libs/ec/cpp/ECMuleSocket.cpp
@@ -59,9 +59,11 @@ bool CECMuleSocket::ConnectSocket(amuleIPV4Address& address)
 // constants used by CECServerSocket on the amuled side so detection
 // is symmetric. Numbers picked to balance responsiveness against the
 // keepalive packet overhead (one probe per 10s after 30s idle).
-namespace { const int EC_KEEPALIVE_IDLE_SEC      = 30; }
-namespace { const int EC_KEEPALIVE_INTERVAL_SEC  = 10; }
-namespace { const int EC_KEEPALIVE_PROBE_COUNT   = 3;  }
+namespace {
+	const int EC_KEEPALIVE_IDLE_SEC      = 30;
+	const int EC_KEEPALIVE_INTERVAL_SEC  = 10;
+	const int EC_KEEPALIVE_PROBE_COUNT   = 3;
+}
 
 bool CECMuleSocket::InternalConnect(uint32_t ip, uint16_t port, bool wait) {
 	amuleIPV4Address addr;
diff --git a/src/libs/ec/cpp/RemoteConnect.cpp b/src/libs/ec/cpp/RemoteConnect.cpp
@@ -31,6 +31,12 @@
 #include "../../../amuleIPV4Address.h"
 
 #include <wx/intl.h>
+#include <common/StringFunctions.h>	// unicode2char for stderr message
+#ifdef __WINDOWS__
+	#include <process.h>		// _exit
+#else
+	#include <unistd.h>		// _exit
+#endif
 
 wxDEFINE_EVENT(wxEVT_EC_CONNECTION, wxEvent);
 CECLoginPacket::CECLoginPacket(const wxString& client, const wxString& version,
@@ -200,10 +206,27 @@ void CRemoteConnect::OnConnect() {
 
 void CRemoteConnect::OnLost() {
 	if (m_notifier) {
-		// Notify app of failure
+		// Notify app of failure — amulegui's wxEvent handler flips the
+		// UI to "disconnected" and stops trying to update.
 		wxECSocketEvent event(wxEVT_EC_CONNECTION,false,_("Connection failure"));
 		m_notifier->AddPendingEvent(event);
+		return;
 	}
+	// Headless EC clients (amulecmd, amuleweb) construct CRemoteConnect
+	// with NULL m_notifier. Continuing to run would mean serving stale
+	// data in amuleweb (HTTP requests still return the template shell
+	// without live amuled data, no error visible to the user) or
+	// sitting at the amulecmd prompt with a dead socket. Failing fast
+	// is the right semantic — supervisor (systemd unit / docker /
+	// shell loop) is the recovery layer and decides whether to restart.
+	fprintf(stderr, "%s\n",
+		(const char *)unicode2char(_("External Connection lost — exiting.")));
+	fflush(stderr);
+	// _exit instead of exit() because the asio worker thread that
+	// just fired this is mid-callback; racing static destructors
+	// against it risks the kind of use-after-free #748 was about.
+	// The OS reaps file descriptors / sockets on exit anyway.
+	_exit(1);
 }
 
 const CECPacket *CRemoteConnect::OnPacketReceived(const CECPacket *packet, uint32 trueSize)
