From cb93927d15bba32059b16edbfa8ad0f9f954479f Mon Sep 17 00:00:00 2001 From: Davide Grilli Date: Tue, 3 Nov 2026 11:45:43 +0100 Subject: [PATCH] fix(net): fail fast on a dead Electrum connection instead of hanging After a protocol error (e.g. a malformed line) the read loop ended but the client still reported IsConnected and later requests waited forever for a response that could never arrive. The read loop now marks the connection closed before failing pending requests, and RequestAsync throws IOException on a closed connection. Adds regression tests and a raw-line helper to the fake server. --- src/Core/Net/ElectrumClient.cs | 12 +++++++ .../Net/ElectrumClientTests.cs | 34 +++++++++++++++++++ .../Net/FakeElectrumServer.cs | 8 +++++ 3 files changed, 54 insertions(+) diff --git a/src/Core/Net/ElectrumClient.cs b/src/Core/Net/ElectrumClient.cs index f1207b9..a5954ba 100644 --- a/src/Core/Net/ElectrumClient.cs +++ b/src/Core/Net/ElectrumClient.cs @@ -120,6 +120,15 @@ public sealed class ElectrumClient : IAsyncDisposable var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _pending[id] = tcs; + // The read loop cancels _cts before failing the pending requests, so a request that + // registers after that point lands here instead of waiting for a response that can + // never arrive (the loop is gone). + if (_cts.IsCancellationRequested) + { + _pending.TryRemove(id, out _); + throw new IOException("Connection to the server closed."); + } + var payload = JsonSerializer.SerializeToUtf8Bytes(new { jsonrpc = "2.0", @@ -212,6 +221,9 @@ public sealed class ElectrumClient : IAsyncDisposable finally { await pipe.CompleteAsync(); + // Mark the connection dead first (IsConnected, RequestAsync, write loop) so nothing + // can register a request after the pending ones are failed below. + try { _cts.Cancel(); } catch (ObjectDisposedException) { } foreach (var (_, tcs) in _pending) tcs.TrySetException(failure ?? new IOException("Connection to the server closed.")); _pending.Clear(); diff --git a/tests/PalladiumWallet.Tests/Net/ElectrumClientTests.cs b/tests/PalladiumWallet.Tests/Net/ElectrumClientTests.cs index 7b1256d..cb1d8bf 100644 --- a/tests/PalladiumWallet.Tests/Net/ElectrumClientTests.cs +++ b/tests/PalladiumWallet.Tests/Net/ElectrumClientTests.cs @@ -98,6 +98,40 @@ public class ElectrumClientTests await disconnected.Task.WaitAsync(Timeout); } + [Fact] + public async Task A_malformed_line_from_the_server_fails_pending_requests_and_reports_the_error() + { + await using var server = new FakeElectrumServer(); + server.Handle("server.banner", _ => FakeElectrumServer.NoResponse); + + await using var client = await ElectrumClient.ConnectAsync(server.Host, server.Port, useSsl: false); + + var disconnected = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + client.Disconnected += ex => disconnected.TrySetResult(ex); + + var pending = client.BannerAsync(); + await server.SendRawAsync("this is not json"); + + await Assert.ThrowsAnyAsync(() => pending.WaitAsync(Timeout)); + Assert.NotNull(await disconnected.Task.WaitAsync(Timeout)); // failure, not a clean close + Assert.False(client.IsConnected); + } + + [Fact] + public async Task Requests_after_the_connection_died_fail_fast_instead_of_hanging() + { + await using var server = new FakeElectrumServer(); + await using var client = await ElectrumClient.ConnectAsync(server.Host, server.Port, useSsl: false); + + var disconnected = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + client.Disconnected += _ => disconnected.TrySetResult(); + await server.SendRawAsync("garbage"); // kills the read loop + await disconnected.Task.WaitAsync(Timeout); + + // The read loop is gone: nobody would ever complete this request. + await Assert.ThrowsAnyAsync(() => client.RequestAsync("server.ping").WaitAsync(Timeout)); + } + [Fact] public async Task Una_risposta_molto_grande_arriva_integra() { diff --git a/tests/PalladiumWallet.Tests/Net/FakeElectrumServer.cs b/tests/PalladiumWallet.Tests/Net/FakeElectrumServer.cs index 5b55e73..898da00 100644 --- a/tests/PalladiumWallet.Tests/Net/FakeElectrumServer.cs +++ b/tests/PalladiumWallet.Tests/Net/FakeElectrumServer.cs @@ -71,6 +71,14 @@ public sealed class FakeElectrumServer : IAsyncDisposable await WriteLineAsync(stream, writeLock, line); } + /// Sends an arbitrary (possibly malformed) line to every connected client. + public async Task SendRawAsync(string line) + { + var bytes = System.Text.Encoding.UTF8.GetBytes(line); + foreach (var (stream, writeLock, _) in _clients) + await WriteLineAsync(stream, writeLock, bytes); + } + /// Abruptly closes every accepted connection (tests the client's failure paths). public void DropClients() {