fix android http cancelation - #142
Open
tx3stn wants to merge 4 commits into
Open
Conversation
|
| Test Suite | Result |
|---|---|
| Valdi Smoke Tests | ❌ failure |
| valdi_web Integration Test | ✅ success |
| macOS: C++ & Platform Tests | ✅ success |
| Snapshot Tests | ✅ success |
| API Surface Check | ✅ success |
| Linux: Build Compiler | ✅ success |
| Linux: C++ Tests | ✅ success |
| Linux: Build & Export | ✅ success |
Some tests failed. Please check the workflow logs for details.
🚀 Bazel remote cache is now enabled - future builds will be faster!
Workflow: Valdi CI
Contributor
Author
|
failing job appears to be a timeout, not related to my changes, but I don't have permissions to re-run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Android
DefaultHTTPRequestManagerdiverges from the iOS/macOS defaults in two ways:Cancellation does nothing.
HTTPRequestTask.cancel()nulls the completion but never touches the connection, so the transfer runs to completion and the result is thrown away.iOS actually cancels the
NSURLSessionTask, so it works there.Requests are serial. The executor is
ThreadPoolExecutor(0, 1, …). iOS usesNSURLSession.sharedSession, which is concurrent by default.So a cancelled request neither stops nor frees the only worker, and everything else queues behind it until it finishes on its own.
Changes
cancel()disconnects the connection (a request cancelled while queued never opens a connection).NSURLSession.HTTPMaximumConnectionsPerHost(so Android behaviour is consistent with iOS)bazel test //valdi:test_java --test_output=errorsis passing.Type of Change
Testing
bazel test //...)Testing Details
Checklist
Related Issues
Additional Context