Merge pull request #784 from frstrtr/ltc/send-socket-deadline

ltc: Send() socket deadline + desync-safe teardown (parity with dash #781)

Author: frstrtr

src/impl/ltc/coin/rpc.cpp

@@ -6,6 +6,12 @@
 
 #include <core/log.hpp>
 #include <core/hash.hpp>
+
+#ifndef _WIN32
+#include <sys/socket.h>          // setsockopt SO_SND/RCVTIMEO (Send() deadline)
+#include <sys/time.h>            // struct timeval
+#endif
+
 namespace ltc
 {
 
@@ -67,6 +73,10 @@ void NodeRPC::connect(NetService address, std::string userpass)
                     {
                         try
                         {
+                            // Bound the synchronous Send() I/O BEFORE check()
+                            // runs its first RPC, so a wedged litecoind cannot
+                            // hang the caller (io thread or thread_pool worker).
+                            apply_socket_timeouts();
                             if (check())
                             {
                                 m_connected = true;
@@ -128,9 +138,51 @@ void NodeRPC::sync_reconnect()
 		LOG_WARNING << "CoindRPC sync_reconnect connect failed: " << ec.message();
 		return;
 	}
+	apply_socket_timeouts();   // re-arm the Send() deadline on the fresh socket
 	LOG_INFO << "CoindRPC reconnected (sync)";
 }
 
+void NodeRPC::close_stream()
+{
+	// Same teardown idiom as sync_reconnect()/the destructor. m_connected is
+	// cleared so the next Send() write-fails into a clean sync_reconnect().
+	beast::error_code ec;
+	m_stream.socket().shutdown(io::ip::tcp::socket::shutdown_both, ec);
+	m_stream.close();
+	m_connected = false;
+}
+
+void NodeRPC::apply_socket_timeouts()
+{
+	// Force the socket back to BLOCKING mode, then set kernel send/receive
+	// timeouts. async_connect (connect()) leaves asio's non_blocking flag set;
+	// under it a synchronous recv returns EAGAIN immediately and SO_RCVTIMEO is
+	// a no-op. In blocking mode the sync http::write/http::read inside Send() do
+	// true blocking send()/recv() which the kernel bounds by SO_SND/RCVTIMEO,
+	// returning an error after RPC_IO_TIMEOUT_SECONDS instead of hanging on a
+	// wedged litecoind/dogecoind. (beast tcp_stream::expires_after cannot be
+	// used: it only governs ASYNC ops -- the sync path calls socket.read_some()
+	// directly.) Per-operation inactivity timeout: a large-but-steadily-arriving
+	// GBT does NOT trip it; only a genuine stall does. POSIX (SO_*TIMEO takes a
+	// timeval); guarded off Windows (no <sys/time.h>/timeval, and Winsock
+	// SO_RCVTIMEO takes a DWORD) -- the deploy target is Linux, and macOS keeps
+	// the real deadline. On Windows this is a no-op (pre-parity behaviour).
+#ifndef _WIN32
+	if (!m_stream.socket().is_open())
+		return;
+	boost::system::error_code ec;
+	m_stream.socket().non_blocking(false, ec);   // guarantee SO_*TIMEO applies
+	if (ec)
+		LOG_WARNING << "CoindRPC: could not set blocking mode: " << ec.message();
+	struct timeval tv;
+	tv.tv_sec  = RPC_IO_TIMEOUT_SECONDS;
+	tv.tv_usec = 0;
+	const int fd = m_stream.socket().native_handle();
+	::setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
+	::setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv));
+#endif
+}
+
 std::string NodeRPC::Send(const std::string &request)
 {
 	std::lock_guard<std::mutex> _rpc_lock(m_rpc_mutex);
@@ -151,6 +203,10 @@ std::string NodeRPC::Send(const std::string &request)
 				sync_reconnect();
 				continue;
 			}
+			// Deadline-desync guard (see the read-fail path below): tear the
+			// socket down before giving up so a partially-written request can
+			// never be answered into a LATER Send() on a reused connection.
+			close_stream();
 			return {};
 		}
 
@@ -169,6 +225,17 @@ std::string NodeRPC::Send(const std::string &request)
 				sync_reconnect();
 				continue;
 			}
+			// Deadline-desync guard (SO_RCVTIMEO deadline hazard): this final
+			// attempt has already WRITTEN the request into litecoind but its
+			// response is unread (read timed out). jsonrpccxx reuses the constant
+			// id "curltest" and never validates response ids, so reusing this
+			// open connection would make the NEXT Send() read THIS request's late
+			// response -> permanent off-by-one desync (a GBT would parse a
+			// getbestblockhash string -> zeroed work -> a stuck "honest set-gap"
+			// outage until the connection churns). Tear the socket down so the
+			// next Send() write-fails into a clean sync_reconnect(). Already
+			// under m_rpc_mutex.
+			close_stream();
 			return {};
 		}
 

src/impl/ltc/coin/rpc.hpp

@@ -58,6 +58,29 @@ class NodeRPC : public jsonrpccxx::IClientConnector
     std::string Send(const std::string &request) override;
     nlohmann::json CallAPIMethod(const std::string& method, const jsonrpccxx::positional_parameter& params = {});
 
+    // Socket deadline (parity with dash #781, impl/dash/coin/rpc.{hpp,cpp}): a
+    // REAL bound on the synchronous Send() I/O so a wedged-but-connected
+    // litecoind/dogecoind (socket open, no bytes) cannot hang the caller forever
+    // -- today that permanently wedges the RPC path (refresh-wedge) AND blocks
+    // join() at shutdown (SIGINT-hang). beast's SYNCHRONOUS read_some/write_some
+    // call socket.read_some() directly and do NOT honour tcp_stream::
+    // expires_after (that timer only fires for ASYNC ops), so the robust bound is
+    // a kernel SO_RCV/SNDTIMEO on the socket forced back to BLOCKING mode
+    // (async_connect leaves asio's non_blocking flag set, under which SO_*TIMEO
+    // is a no-op). apply_socket_timeouts() does both; called after every
+    // successful (re)connect. Linux release target; no-op on Windows (pre-parity
+    // behaviour: no deadline).
+    static constexpr int RPC_IO_TIMEOUT_SECONDS = 12;
+    void apply_socket_timeouts();
+    // Tear down m_stream (sync_reconnect idiom). Called on a FINAL-attempt Send()
+    // failure so a half-written / unread request can never be answered into a
+    // later Send() on a reused connection -- the deadline-desync guard. The
+    // constant "curltest" id (ID above) + jsonrpccxx not validating response ids
+    // make any reused-connection off-by-one a permanent set-gap outage (a GBT
+    // would parse a getbestblockhash string -> zeroed work). Caller holds
+    // m_rpc_mutex.
+    void close_stream();
+
 public:
     NodeRPC(io::io_context* context, ltc::interfaces::Node* coin, bool testnet);
     ~NodeRPC();