[swift5] URLSession: Fix memory leak of SessionDelegate#8558
Merged
wing328 merged 3 commits intoOpenAPITools:masterfrom Jan 29, 2021
Merged
[swift5] URLSession: Fix memory leak of SessionDelegate#8558wing328 merged 3 commits intoOpenAPITools:masterfrom
wing328 merged 3 commits intoOpenAPITools:masterfrom
Conversation
Contributor
Author
4brunu
approved these changes
Jan 28, 2021
Contributor
4brunu
left a comment
There was a problem hiding this comment.
Looks good to me.
Thanks for your contribution.
| } | ||
|
|
||
| let cleanupRequest = { | ||
| urlSessionStore[urlSessionId]?.finishTasksAndInvalidate() |
Contributor
There was a problem hiding this comment.
Interesting, I didn't know this API.
Can you please comment on the need of this?
Just curious.
Thanks 😊
Contributor
Author
There was a problem hiding this comment.
This invalidates the session after the all tasks of this session have completely finished. New tasks cannot be started on this session.
This call is not necessary, but good practice, since we're creating a session for each request, so after a task as finished we don't need the session anymore.
Contributor
|
For me it's ready to merge 👍 |
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.
cleanupRequest()is only called on error for now. SosessionDelegatedoesn't get deallocated after request.I think it's okay to call
cleanupRequest()directly after the processing of the response, instead of putting it in the completion closure, since it will only deallocate when not in use anymore. What do you guys think?PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.For Windows users, please run the script in Git BASH.
master,5.1.x,6.0.x