-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add default implementations for three default protocol conformances in the URLSessionDelegate family #5022
Add default implementations for three default protocol conformances in the URLSessionDelegate family #5022
Conversation
@swift-ci please test |
@jrflat to review (and also to verify behavior compatibility with Darwin) |
I think at the very least we need test cases to cover the desired new behavior. |
Sources/FoundationNetworking/URLSession/URLSessionDelegate.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this! These default behaviors all LGTM and match the Darwin behavior. I just left one comment about needing to call both the task and session delegate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking as request changes per @jrflat feedback.
…n the URLSessionDelegate family Implementing these callbacks without calling the completion handler causes hangs in cases where these methods are called. Add reasonable default behaviors for all of them, to prevent this. This issue has been in place for at least 8 years for one of these callbacks.
d43a53f
to
3420746
Compare
Thanks for the review @jrflat! Does this look right, now? |
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
@swift-ci please test Windows platform |
Implementing these callbacks without calling the completion handler causes hangs in cases where these methods are called. Add reasonable default behaviors for all of them, to prevent this.
This issue has been in place for at least 8 years for one of these callbacks.