Fix AndroidMessageHandler response body cancellation by simonrozsival · Pull Request #11554 · dotnet/android

simonrozsival · 2026-06-01T12:40:40Z

Fixes #11548

Summary

make response-content reads cancellation-aware in AndroidMessageHandler
abort stalled body reads by disconnecting the HttpURLConnection and disposing the response stream when the read token is canceled
add dedicated HttpListener-based coverage for ResponseContentRead and ResponseHeadersRead body cancellation

Testing

make all
ANDROID_SERIAL=R58Y30HZ65V ./dotnet-local.sh build -t:RunTestApp tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj -p:IncludeCategories=AndroidMessageHandlerCancellation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival · 2026-06-01T13:32:05Z

/review

github-actions · 2026-06-01T13:32:41Z

✅ Android PR Reviewer completed successfully!

Copilot

Pull request overview

This PR fixes delayed cancellation observation when AndroidMessageHandler is blocked reading the response body (after headers are received). It does so by wrapping the response stream to actively disconnect/close the underlying HttpURLConnection when the read cancellation token is canceled, and adds regression coverage for both ResponseContentRead and ResponseHeadersRead scenarios (body read canceled after headers).

Changes:

Add a cancellation-aware response body stream wrapper that aborts stalled reads by calling HttpURLConnection.Disconnect() and disposing the response stream when cancellation is requested.
Wire the wrapped stream into AndroidMessageHandler response content creation.
Add HttpListener-based device tests covering prompt body-read cancellation for both completion options.

Show a summary per file

File	Description
`src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs`	Wraps response content stream to make body reads cancellation-aware by aborting the underlying Java connection/stream.
`tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerCancellationTests.cs`	Adds new tests verifying response body cancellation is observed promptly for `ResponseContentRead` and `ResponseHeadersRead`.
`tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj`	Includes the new cancellation test file in the test project build.

Copilot's findings

Files reviewed: 3/3 changed files
Comments generated: 1

github-actions

⚠️ Needs Changes

Summary: This PR adds cancellation support for response body reads in AndroidMessageHandler by wrapping the response stream in a CancellationAwareResponseStream that disconnects the HttpURLConnection when a CancellationToken is canceled. The approach is sound — registering a cancellation callback to force-close the connection is the right pattern for unblocking stalled Java I/O.

Issues found:

Severity	Count	Category
⚠️ warning	2	Resource management, Thread safety
💡 suggestion	2	Testing

Key concern: The CancellationAwareResponseStream takes dispose ownership of httpConnection, but AndroidHttpResponseMessage also disposes the same connection instance — creating double-dispose and unclear ownership. The AbortRead callback and Dispose method also race on stream.Dispose().

Positive callouts:

Clean separation of the cancellation wrapper as a nested class
Good use of CancellationToken.Register + exception filter pattern
The ShouldMapToCancellation exception mapping covers the right exception types from the Java interop layer
Well-structured test server with proper synchronization via TaskCompletionSource
Tests cover both ResponseContentRead and ResponseHeadersRead completion options

Generated by Android PR Reviewer for issue #11554 · ● 14.1M

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival · 2026-06-01T14:53:39Z

/review

github-actions · 2026-06-01T14:54:17Z

✅ Android PR Reviewer completed successfully!

github-actions

✅ LGTM — Clean, well-designed fix

Summary: The PR wraps the response stream in a CancellationAwareResponseStream that registers a cancellation callback to disconnect the HttpURLConnection and dispose the stream, unblocking stalled reads. The approach is sound — it correctly handles the ReadAsync/CopyToAsync paths that StreamContent actually uses, maps the resulting exceptions to OperationCanceledException, and uses Interlocked for thread-safe double-dispose protection.

Positive callouts:

The QueueAbortRead → Task.Run(AbortRead) pattern avoids blocking the cancellation callback thread — good attention to detail
TaskCreationOptions.RunContinuationsAsynchronously on the TaskCompletionSource prevents deadlocks in the test server
The test infrastructure (StalledResponseServer) is well-structured with proper cleanup in TearDown
Good coverage of both ResponseContentRead and ResponseHeadersRead completion options

CI: dotnet-android ✅ passed. Xamarin.Android-PR not yet visible (expected for a recently pushed direct PR).

Issues: 0 ❌ · 0 ⚠️ · 3 💡

Generated by Android PR Reviewer for issue #11554 · ● 12.5M

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot · 2026-06-02T09:20:07Z

@copilot fix this build failure:

simonrozsival · 2026-06-02T12:31:40Z

@copilot fix the build error:

All test configurations in both the Package Tests and MSBuild Emulator Tests stages fail due to a single compile error:

error CS0102: The type 'AndroidMessageHandlerCancellationTests' already contains a definition for 'InitialResponseChunk'
File: tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerCancellationTests.cs (line 27)

The field or property InitialResponseChunk is declared twice in the AndroidMessageHandlerCancellationTests class, causing Mono.Android.NET-Tests.csproj to fail to compile.

Recommended Fix
Remove the duplicate definition of InitialResponseChunk in AndroidMessageHandlerCancellationTests.cs at line 27.

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot · 2026-06-02T12:55:38Z

@copilot fix the build error:

All test configurations in both the Package Tests and MSBuild Emulator Tests stages fail due to a single compile error:
...

Fixed in 5494513.

I removed the duplicate InitialResponseChunk definition from AndroidMessageHandlerCancellationTests.cs and kept the remaining initializer as [42].

jonathanpeppers · 2026-06-02T14:47:42Z

+			public override int Read (byte[] buffer, int offset, int count)
+			{
+				return stream.Read (buffer, offset, count);
+			}


There are several methods in here, we could shorten to a single line using =>.

jonathanpeppers · 2026-06-02T14:48:19Z

+					try {
+						await stream.CopyToAsync (destination, bufferSize, cancellationToken).ConfigureAwait (false);
+					} catch (Exception ex) when (ShouldMapToCancellation (ex, cancellationToken)) {
+						throw new global::System.OperationCanceledException ("Response body read was canceled.", ex, cancellationToken);


does this really need global::, is there a second OperationCanceledException?

jonathanpeppers · 2026-06-02T14:50:14Z

+			void QueueAbortRead ()
+			{
+				Task.Run (AbortRead).ContinueWith (
+					LogAbortReadException,


This method is used in one place, could it be a lambda instead?

jonathanpeppers · 2026-06-02T14:50:49Z

/review

github-actions · 2026-06-02T14:51:24Z

✅ Android PR Reviewer completed successfully!

github-actions

⚠️ Needs Changes

Nice fix for response body cancellation — the CancellationAwareResponseStream approach with Disconnect() + stream disposal is a clean way to unblock stalled reads. The test design with a stalled HTTP server is well thought out.

Issues found

Severity	Count
❌ Error	1
💡 Suggestion	2

❌ Missing field declaration — stalledResponseServer is used but never declared in the test class. This will fail to compile.

💡 Per-read CancellationToken.Register — The registration/disposal happens on every ReadAsync call in a tight loop. Consider registering once and disposing in Dispose.

💡 Test coverage — Consider a positive test (non-canceled read completes normally) to verify the wrapper doesn't break the happy path.

CI Status

✅ dotnet-android — succeeded
✅ license/cla — succeeded
⏳ Xamarin.Android-PR — not yet reported (will likely surface the compile error above)

Generated by Android PR Reviewer for issue #11554 · ● 17.4M

github-actions · 2026-06-02T14:57:38Z

+		const int BodyReadBlockDelayMilliseconds = 250;
+		const int PromptCancellationTimeoutMilliseconds = 3000;
+
+		static readonly byte[] InitialResponseChunk = [42];


🤖 ❌ Bugs & correctness — The stalledResponseServer instance field is used in SetUp, TearDown, and both test methods, but it is never declared. This will be a compile error. Add the field declaration:

Suggested change

static readonly byte[] InitialResponseChunk = [42];

static readonly byte[] InitialResponseChunk = [42];

StalledResponseServer? stalledResponseServer;

Rule: Missing field declaration

github-actions · 2026-06-02T14:57:38Z

+			{
+				cancellationToken.ThrowIfCancellationRequested ();
+
+				using (cancellationToken.Register (QueueAbortRead, useSynchronizationContext: false)) {


🤖 💡 Performance — The cancellationToken.Register call allocates a CancellationTokenRegistration on every ReadAsync invocation. For StreamContent draining a response body, this is called in a tight loop (once per buffer-sized chunk). Consider registering the callback once (e.g., in the constructor or via a lazy field) and disposing it in Dispose, rather than registering/unregistering per read.

Rule: Avoid closures in hot paths

github-actions · 2026-06-02T14:57:38Z

+				releaseBody.TrySetResult (true);
+			}
+
+			async Task ServeStalledResponseBody ()


🤖 💡 Testing — The StalledResponseServer only handles a single HTTP request (GetContextAsync is called once). If a future test reuses the server for multiple requests, it will hang silently. A comment noting this single-request design would help future maintainers.

Also, consider adding a negative test: verify that a non-canceled read completes successfully through the CancellationAwareResponseStream (i.e., the wrapper doesn't break normal reads).

Rule: Test edge cases

simonrozsival · 2026-06-02T21:59:14Z

@copilot address code reviews

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot · 2026-06-02T22:42:22Z

@copilot address code reviews

Addressed in 982e626.

simonrozsival and others added 2 commits June 1, 2026 14:40

Fix AndroidMessageHandler response body cancellation

f20a13d

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Simplify AndroidMessageHandler cancellation changes

385d14f

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival marked this pull request as ready for review June 1, 2026 13:31

simonrozsival requested a review from jonathanpeppers as a code owner June 1, 2026 13:31

Copilot AI review requested due to automatic review settings June 1, 2026 13:31

Copilot started reviewing on behalf of simonrozsival June 1, 2026 13:31 View session

Copilot AI reviewed Jun 1, 2026

View reviewed changes

Comment thread src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs

github-actions Bot reviewed Jun 1, 2026

View reviewed changes

Address AndroidMessageHandler review feedback

bf88397

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread ...droid-Tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerCancellationTests.cs

Comment thread src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs

Comment thread src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs

Apply suggestions from code review

f520475

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

This comment was marked as resolved.

Sign in to view

Copilot started work on behalf of simonrozsival June 2, 2026 08:51 View session

Fix duplicate cancellation helper body

ad6951b

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot finished work on behalf of simonrozsival June 2, 2026 09:20

Copilot started work on behalf of simonrozsival June 2, 2026 12:31 View session

Copilot AI and others added 2 commits June 2, 2026 12:54

Remove duplicate test field

1a2b506

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Use collection expression in test field

5494513

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot finished work on behalf of simonrozsival June 2, 2026 12:56

jonathanpeppers reviewed Jun 2, 2026

View reviewed changes

github-actions Bot requested changes Jun 2, 2026

View reviewed changes

Copilot started work on behalf of simonrozsival June 2, 2026 21:59 View session

Address AndroidMessageHandler review feedback

982e626

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot finished work on behalf of simonrozsival June 2, 2026 22:42

Conversation

simonrozsival commented Jun 1, 2026

Summary

Testing

Uh oh!

simonrozsival commented Jun 1, 2026

Uh oh!

github-actions Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

⚠️ Needs Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Jun 1, 2026

Uh oh!

github-actions Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

✅ LGTM — Clean, well-designed fix

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Copilot AI commented Jun 2, 2026

@copilot fix this build failure:

Uh oh!

simonrozsival commented Jun 2, 2026

Uh oh!

Copilot AI commented Jun 2, 2026

Uh oh!

jonathanpeppers Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers commented Jun 2, 2026

Uh oh!

github-actions Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

⚠️ Needs Changes

Issues found

CI Status

Uh oh!

github-actions Bot Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

simonrozsival commented Jun 2, 2026

Uh oh!

Copilot AI commented Jun 2, 2026

Uh oh!

Reviewers

Assignees

Labels

github-actions Bot commented Jun 1, 2026 •

edited

Loading

github-actions Bot commented Jun 1, 2026 •

edited

Loading

github-actions Bot commented Jun 2, 2026 •

edited

Loading