Skip to content

Allow NSURLSessions to be reused on Apple platforms#1006

Open
lawrencewuskyboxlabs wants to merge 1 commit into
mainfrom
user/lawrencewu/nsurl_shared_session
Open

Allow NSURLSessions to be reused on Apple platforms#1006
lawrencewuskyboxlabs wants to merge 1 commit into
mainfrom
user/lawrencewu/nsurl_shared_session

Conversation

@lawrencewuskyboxlabs

Copy link
Copy Markdown
Collaborator

The current implementation of AppleHttpProvider will spin up a new http_task_apple and thus a new NSURLSession with each call that is performed. This somewhat goes against Apple's recommended practice of reusing the sessions when possible and is contributing to out of memory issues in cases where a high number of concurrent calls are active, along with possibly another memory leak due to the sessions not being manually invalidated. These changes rework the Provider and the SessionDelegate to allow the same session to be used for calls that share the same timeout period, as this is the only difference in the currently-created session configs.

Key changes:

  • A new AppleHttpSessionManager has been introduced which largely replaces the functionality provided by the now-deleted http_task_apple. Rather than individual http_tasks being created, there is only one Manager which is responsible for creating sessions, creating and running NSURLSessionTasks on the appropriate sessions, and closing sessions when all their tasks are completed. Sessions are represented by AppleHttpSessions and are identified by their timeout period, and each session also has a corresponding map of AppleHttpTaskContexts keyed by NSUInteger task identifiers. This is to keep track of the HCCallHandle and XAsyncBlock that should be used for each task, since originally each http_task would have this info.
  • The AppleHttpProvider owns the Manager through a shared pointer and is essentially a wrapper around the Manager, due to the required opaqueness of the NSObjects the Manager has to use. The shared pointer is to allow weak pointers to be safely made.
  • Each session still has its own SessionDelegate, but since there can now be multiple tasks in the session, the Delegate also maintains its own map of TaskContexts keyed by task identifiers. On a per-task basis, it needs to track the call handle and, for reporting purposes, the expected download size. The call handle can be given by the Manager after a task is created since the Manager is also responsible for starting the task. The download size can be cached upon the initial reply the task receives.
  • The completion handler block make use of the aforementioned weak pointers to the Manager.
  • The old _dataToDownload in the Delegate, which was only used to report the amount of bytes downloaded so far, is deemed unnecessary because the call handle's responseBodyBytes should match it after it's written to.
  • With the removal of http_task, the call handle no longer has context set by the Provider. This is considered to prevent another possible memory leak in the old implementation, where the release of the http_task in the Provider meant it was up to the call handle to clean up the http_task. Apple and Android are the only platforms in libhttpclient that have the context set so this also brings Apple more in line with the majority.

I'm open to any feedback on this, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant