[SignalR] [Java] Fix hang when reading an HTTP response body fails by BrennanConroy · Pull Request #69037 · dotnet/aspnetcore

BrennanConroy · 2026-09-03T21:39:45Z

Fixes a hang where HubConnection.start() never completes and the connection cannot be recovered or disposed.

Description

DefaultHttpClient.send read the response body inside OkHttp's onResponse callback:

public void onResponse(Call call, Response response) throws IOException {
    try (ResponseBody body = response.body()) {
        HttpResponse httpResponse = new HttpResponse(response.code(), response.message(), ByteBuffer.wrap(body.bytes()));
        responseSubject.onSuccess(httpResponse);
    }   // body.bytes() throws -> nothing terminates responseSubject
}

OkHttp sets signalledCallback = true before invoking onResponse, so anything thrown from there is only logged (Callback failure for call to ...) and onFailure is never called. The SingleSubject is left unterminated.

Nothing bounds that wait. Only the handshake has a SignalR-level timeout (15s), and it is armed after transport.send() returns; negotiate and transport startup rely solely on OkHttp's per-operation timeouts. So a negotiate response that fails mid-body leaves start() pending forever:

The Completable returned by start() never terminates, and subsequent start() calls return that same incomplete task.
stop() hangs, because it waits on connectionState.startTask.
close() hangs for the same reason, so its finally never reaches httpClient.close() and the OkHttp dispatcher threads are never shut down.

The trigger needs no misconfiguration — response headers arrive, then the body read fails (connection reset, proxy/load balancer dropping the connection, truncated body). Setting callTimeout makes it more likely rather than less: when the deadline lands during the body read, the resulting InterruptedIOException is exactly what gets swallowed, turning a transient stall into a permanent wedge.

This has been present since the code was introduced (#25253, 2020), so it affects all shipped versions.

The fix

Terminate the Single when reading the body fails. onSuccess also moves out of the try block so that a throwing downstream observer can no longer double-terminate the subject.

DefaultHttpClient is package-private, so there is no public API change.

Verification

New DefaultHttpClientTest uses a plain ServerSocket to serve a truncated response, so no new test dependencies are needed.

Test	Before	After
`requestCompletesWhenResponseIsWellFormed`	passed	passed
`requestFailsWhenResponseBodyIsTruncated`	failed — "The request never terminated"	passed
`startFailsWhenNegotiateResponseIsTruncated`	failed — `expected: <DISCONNECTED> but was: <CONNECTING>`	passed

Both regression tests were confirmed to fail without the fix for the bug's actual reason, and the happy-path test stays green, so they are not passing incidentally. The end-to-end test awaits the start task directly instead of wrapping it in .timeout(...), which would otherwise report success while start() was still hung.

Full Java client suite: 295 tests, 0 failures.

OkHttp sets `signalledCallback` before invoking `onResponse`, so it only logs anything thrown from there and never calls `onFailure`. `DefaultHttpClient` read the response body inside `onResponse`, so when `body.bytes()` threw the `SingleSubject` was never terminated. Nothing bounds that wait: only the handshake has a SignalR-level timeout, and it is armed after the transport starts. A negotiate response that fails mid-body therefore left `HubConnection.start()` pending forever. `stop()` and `close()` also hung, because both wait on the start task, so the connection could not be recovered or disposed. Terminate the Single when reading the body fails. `onSuccess` also moves out of the try block so a throwing downstream observer cannot double-terminate it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

There are a couple of correctness/reliability nits in the changed code (notably catching Throwable and a non-daemon per-connection test thread) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 2

New issues introduced by this change (2)

Severity	Finding
	`src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/DefaultHttpClient.java` — Avoid catching Throwable here. Catching Throwable will also intercept Error (e.g.,…
	`src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/DefaultHttpClientTest.java` — The per-connection response thread is not marked daemon. If the client stalls before sending a…

What changed in this PR

This PR fixes a hang in the Java SignalR client where HubConnection.start() (and subsequently stop()/close()) could wait forever if reading an HTTP response body fails inside OkHttp’s onResponse callback, by ensuring the SingleSubject is always terminated.

Changes:

Ensure DefaultHttpClient terminates the SingleSubject with onError when response-body reading fails inside onResponse.
Add regression tests using a raw ServerSocket server that intentionally truncates the HTTP response body to reproduce the hang and verify the fix.

File	Description
src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/DefaultHttpClient.java	Terminates the `SingleSubject` on exceptions during response-body read to prevent `start()` from hanging indefinitely.
src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/DefaultHttpClientTest.java	Adds targeted tests (well-formed vs truncated response) to validate termination behavior and prevent regressions.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

                try (ResponseBody body = response.body()) {
-                    HttpResponse httpResponse = new HttpResponse(response.code(), response.message(), ByteBuffer.wrap(body.bytes()));
-                    responseSubject.onSuccess(httpResponse);
+                    httpResponse = new HttpResponse(response.code(), response.message(), ByteBuffer.wrap(body.bytes()));
+                } catch (Throwable ex) {
+                    // OkHttp marks the callback as signalled before invoking onResponse, so it never calls
+                    // onFailure for anything thrown from here. Reading the body can still fail, for example when


+                        Socket socket = serverSocket.accept();
+                        new Thread(() -> respond(socket, mode)).start();


halter73 · 2026-09-09T00:09:05Z

+                    // onFailure for anything thrown from here. Reading the body can still fail, for example when
+                    // the connection drops mid-response, so terminate the Single ourselves. Otherwise callers
+                    // such as negotiate would wait on it forever.
+                    responseSubject.onError(ex);


A truncated response to the long-polling handshake POST now fails start(), but the transport keeps polling. The startup error handler marks the hub disconnected without cleaning up, so a subsequent stop() is a no-op and retries can accumulate abandoned transports. A WebSocket handshake timeout has the same cleanup problem.

Can we release the failed attempt's transport and timers while preserving the original startup error?

halter73 · 2026-09-09T00:09:05Z

+            observer.assertError(IOException.class);
+            assertEquals(HubConnectionState.DISCONNECTED, hubConnection.getConnectionState());
+
+            hubConnection.stop().timeout(30, TimeUnit.SECONDS).blockingAwait();


There is still a stop() hang if it's called while the initial long-poll GET is pending and that response is truncated. Startup now fails, but the waiting stop proceeds into LongPollingTransport.stop(), which waits for receiveLoopSubject after DELETE succeeds. The receive loop never started, so nothing completes that subject.

Can we explicitly handle failure before the receive loop starts so stop doesn't wait for a nonexistent loop? We should cover this with a regression that calls stop() before failing the initial GET and lets DELETE succeed. Calling stop only after startup has failed misses this path.

halter73

My two comments are nonblocking for this PR, which is focused on preventing start() from hanging. It's worth filing follow-up issues to ensure startup failures release resources and don't leave stop() hanging, especially now that these failures will be reported instead of leaving start() pending.

BrennanConroy · 2026-09-10T20:44:17Z

/ba-g unrelated flaky blazor test #68856

BrennanConroy · 2026-09-10T20:45:05Z

/backport to release/11.0

github-actions · 2026-09-10T20:45:14Z

Started backporting to release/11.0 (link to workflow run)

BrennanConroy · 2026-09-10T20:45:14Z

/backport to release/10.0

BrennanConroy · 2026-09-10T20:45:22Z

/backport to release/9.0

github-actions · 2026-09-10T20:45:23Z

Started backporting to release/10.0 (link to workflow run)

BrennanConroy · 2026-09-10T20:45:30Z

/backport to release/8.0

github-actions · 2026-09-10T20:45:32Z

Started backporting to release/9.0 (link to workflow run)

github-actions · 2026-09-10T20:45:38Z

Started backporting to release/8.0 (link to workflow run)

…69037) (#69209) Co-authored-by: Brennan <brecon@microsoft.com>

…69037) (#69208) Co-authored-by: Brennan <brecon@microsoft.com>

…69037) (#69207) Co-authored-by: Brennan <brecon@microsoft.com>

…69037) (#69206) Co-authored-by: Brennan <brecon@microsoft.com>

@DeagleGross

* [SignalR] Fix CTS disposal (#69158) * Fix: hardening http2 to refuse newline chars on trailers and dynamic HPACK table (#68743) * Hardening trailers to refuse new line chars * Hardening HPACK dynamic header to refuse new line chars * Http3 request trailers with newline chars test * Added test to exercise newline chars on indexed headers * Fix flaky HTTP/3 request trailers test Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * [SignalR] [Java] Fix hang when reading an HTTP response body fails (#69037) (#69206) Co-authored-by: Brennan <brecon@microsoft.com> * Add AggressiveOptimization to key long-running async methods (#69287) * [DirectTLS] [release/11.0] exclude experimental transport from release 11 (#69144) * Revert "Run DirectTls handshake user callbacks off the epoll pump thread (#68664)" This reverts commit 49e0516. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Revert "[Experimental] DirectTls transport implementation (#67912)" This reverts commit 07d11eb. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Apply suggestion from @DeagleGross --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Update dependencies from https://github.com/dotnet/extensions build 20260909.4 (#69269) On relative base path root Microsoft.Extensions.Caching.Hybrid , Microsoft.Extensions.Diagnostics.Testing , Microsoft.Extensions.ServiceDiscovery , Microsoft.Extensions.ServiceDiscovery.Yarp , Microsoft.Extensions.TimeProvider.Testing From Version 10.9.0 -> To Version 10.10.0 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * [release/11.0] Source code updates from dotnet/dotnet (#69256) * Backflow from https://github.com/dotnet/dotnet / e775569 build 331727 Diff: https://github.com/dotnet/dotnet/compare/48e3f46c03e3ed36611a49fd7107330dade9f7c8..e775569ca8ce94c363d910311f5f889e648ddc23 From: dotnet/dotnet@48e3f46 To: dotnet/dotnet@e775569 [[ commit created by automation ]] * Update dependencies from build 331727 Updated Dependencies: Microsoft.NET.Runtime.WebAssembly.Sdk, Microsoft.NETCore.BrowserDebugHost.Transport, Microsoft.NET.Runtime.MonoAOTCompiler.Task, dotnet-ef, Microsoft.Bcl.AsyncInterfaces, Microsoft.Bcl.Cryptography, Microsoft.Bcl.TimeProvider, Microsoft.EntityFrameworkCore, Microsoft.EntityFrameworkCore.Design, Microsoft.EntityFrameworkCore.InMemory, Microsoft.EntityFrameworkCore.Relational, Microsoft.EntityFrameworkCore.Sqlite, Microsoft.EntityFrameworkCore.SqlServer, Microsoft.EntityFrameworkCore.Tools, Microsoft.Extensions.Caching.Abstractions, Microsoft.Extensions.Caching.Memory, Microsoft.Extensions.Configuration, Microsoft.Extensions.Configuration.Abstractions, Microsoft.Extensions.Configuration.Binder, Microsoft.Extensions.Configuration.CommandLine, Microsoft.Extensions.Configuration.EnvironmentVariables, Microsoft.Extensions.Configuration.FileExtensions, Microsoft.Extensions.Configuration.Ini, Microsoft.Extensions.Configuration.Json, Microsoft.Extensions.Configuration.UserSecrets, Microsoft.Extensions.Configuration.Xml, Microsoft.Extensions.DependencyInjection, Microsoft.Extensions.DependencyInjection.Abstractions, Microsoft.Extensions.DependencyModel, Microsoft.Extensions.Diagnostics, Microsoft.Extensions.Diagnostics.Abstractions, Microsoft.Extensions.FileProviders.Abstractions, Microsoft.Extensions.FileProviders.Composite, Microsoft.Extensions.FileProviders.Physical, Microsoft.Extensions.FileSystemGlobbing, Microsoft.Extensions.HostFactoryResolver.Sources, Microsoft.Extensions.Hosting, Microsoft.Extensions.Hosting.Abstractions, Microsoft.Extensions.Http, Microsoft.Extensions.Logging, Microsoft.Extensions.Logging.Abstractions, Microsoft.Extensions.Logging.Configuration, Microsoft.Extensions.Logging.Console, Microsoft.Extensions.Logging.Debug, Microsoft.Extensions.Logging.EventLog, Microsoft.Extensions.Logging.EventSource, Microsoft.Extensions.Logging.TraceSource, Microsoft.Extensions.Options, Microsoft.Extensions.Options.ConfigurationExtensions, Microsoft.Extensions.Options.DataAnnotations, Microsoft.Extensions.Primitives, Microsoft.Internal.Runtime.AspNetCore.Transport, Microsoft.NETCore.App.Ref, Microsoft.NETCore.Platforms, System.Collections.Immutable, System.Composition, System.Configuration.ConfigurationManager, System.Diagnostics.DiagnosticSource, System.Diagnostics.EventLog, System.Diagnostics.PerformanceCounter, System.DirectoryServices.Protocols, System.Formats.Asn1, System.Formats.Cbor, System.IO.Hashing, System.IO.Pipelines, System.Memory.Data, System.Net.Http.Json, System.Net.Http.WinHttpHandler, System.Net.ServerSentEvents, System.Numerics.Tensors, System.Reflection.Metadata, System.Resources.Extensions, System.Runtime.Caching, System.Security.Cryptography.Pkcs, System.Security.Cryptography.Xml, System.Security.Permissions, System.ServiceProcess.ServiceController, System.Text.Encodings.Web, System.Text.Json, System.Threading.AccessControl, System.Threading.Channels, System.Threading.RateLimiting (Version 11.0.0-rc.2.26458.109 -> 11.0.0-rc.2.26461.110) Microsoft.DotNet.Arcade.Sdk, Microsoft.DotNet.Build.Tasks.Archives, Microsoft.DotNet.Build.Tasks.Installers, Microsoft.DotNet.Build.Tasks.Templating, Microsoft.DotNet.Helix.JobMonitor, Microsoft.DotNet.Helix.Sdk, Microsoft.DotNet.RemoteExecutor, Microsoft.DotNet.SharedFramework.Sdk (Version 11.0.0-beta.26458.109 -> 11.0.0-beta.26461.110) Microsoft.Web.Xdt (Version 3.3.0-rc.2.26458.109 -> 3.3.0-rc.2.26461.110) NuGet.Frameworks, NuGet.Packaging, NuGet.Versioning (Version 7.12.0-rc.45909 -> 7.12.0-rc.46210) [[ commit created by automation ]] * Fix authorization component Razor compilation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(iis): scope upgrade MaxRequestBodySize assert to in-process hosting - in-process must clear the limit on upgrade because its read loop enforces it - out-of-process runs on Kestrel, which never enforces it once upgraded - unblocks the three WebSocketsOutOfProcessTests failures 🧪 - Generated by Copilot (cherry picked from commit befb2e9) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: William Godbe <wigodbe@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Roman Konecny <rokonecn@microsoft.com> * Remove experimental DirectTLS transport Carry the release/11.0 removal from #69144 into main while preserving the other auto-merge conflict resolutions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Brennan <brecon@microsoft.com> Co-authored-by: Alan Morais Fialho <51328556+amf-fs@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com> Co-authored-by: Sam Patel <46026722+SamMonoRT@users.noreply.github.com> Co-authored-by: Korolev Dmitry <dmkorolev@microsoft.com> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: William Godbe <wigodbe@microsoft.com> Co-authored-by: Roman Konecny <rokonecn@microsoft.com>

Copilot AI lite review requested due to automatic review settings September 3, 2026 21:39

BrennanConroy requested a review from halter73 as a code owner September 3, 2026 21:39

github-actions Bot added the area-signalr Includes: SignalR clients and servers label Sep 3, 2026

Copilot started reviewing on behalf of BrennanConroy September 3, 2026 21:40

View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

halter73 reviewed Sep 9, 2026

View reviewed changes

halter73 approved these changes Sep 10, 2026

View reviewed changes

BrennanConroy merged commit 8677888 into main Sep 10, 2026
31 of 33 checks passed

BrennanConroy deleted the brennanconroy-java-signalr-start-hang branch September 10, 2026 20:44

BrennanConroy mentioned this pull request Sep 10, 2026

[SignalR] [Java] Fix hung start/stop after a failed connection attempt #69213

Draft

dotnet-maestro Bot mentioned this pull request Sep 11, 2026

[main] Source code updates from dotnet/aspnetcore dotnet/dotnet#9261

Merged

dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Sep 11, 2026

wtgodbe pushed a commit that referenced this pull request Sep 11, 2026

[SignalR] [Java] Fix hang when reading an HTTP response body fails (#…

6d7a8e9

…69037) (#69209) Co-authored-by: Brennan <brecon@microsoft.com>

wtgodbe pushed a commit that referenced this pull request Sep 11, 2026

[SignalR] [Java] Fix hang when reading an HTTP response body fails (#…

8be48f2

…69037) (#69208) Co-authored-by: Brennan <brecon@microsoft.com>

wtgodbe pushed a commit that referenced this pull request Sep 11, 2026

[SignalR] [Java] Fix hang when reading an HTTP response body fails (#…

1307f36

…69037) (#69207) Co-authored-by: Brennan <brecon@microsoft.com>

wtgodbe pushed a commit that referenced this pull request Sep 11, 2026

[SignalR] [Java] Fix hang when reading an HTTP response body fails (#…

b2374cd

…69037) (#69206) Co-authored-by: Brennan <brecon@microsoft.com>

		Socket socket = serverSocket.accept();
		new Thread(() -> respond(socket, mode)).start();

Conversation

BrennanConroy commented Sep 3, 2026

Description

The fix

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

halter73 Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

halter73 Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

halter73 left a comment

Choose a reason for hiding this comment

Uh oh!

BrennanConroy commented Sep 10, 2026

Uh oh!

Uh oh!

BrennanConroy commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

BrennanConroy commented Sep 10, 2026

Uh oh!

BrennanConroy commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

BrennanConroy commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants