libs-base
libs-base copied to clipboard
gnustep
NSURLSession Reimplementation
This Pull Request includes a complete reimplementation of the NSURLSession API.
Issues with the current implementation:
- Incorrect implementation of the libcurl Multi Socket API (Timer Resetting etc.)
- To much indirections (At least 9 custom Classes just for transfer setup)
- Linear search to associate handles with wrapper instances
- Poor error descriptions
- Hard to maintain and debug
- Missing features such as NSURLSessionUploadTask, Metrics, etc.
- Session and tasks were never released due to Retain Cycle in Block Dispose Function
I reduced the implementation to just two main classes: NSURLSession and NSURLSessionTask.
NSURLSession manages a libcurl multi handle and the required plumming via libdispatch. Timeout and socket actions are handled in the libdispatch work queue. Delegate messages and completion handlers are submitted to a seperate serial NSOperationQueue.
How a typical transfer works
Session Setup
When creating a new session, a libcurl multi handle is created and configured according to the NSURLSessionConfiguration object that was passed as an argument.
A serial dispatch queue is created for handling libcurl timers (dispatch_timer) and socket events (via dispatch_io).
The libcurl multi handle delegates all plumming the the application. This means we can drive the full transfer via libdispatch. libcurl instructs the application to set up timers. A call to a libcurl update function is made when the timer fires.
Task Setup
The task object is first initialised by the session (i.e -[NSURLSession dataTaskWithRequest:]). When creating the NSURLSessionTask, a new libcurl easy handle is created and configured with the settings from NSURLRequest and NSURLSessionConfiguration. Before returning the newly created task object, the session configures the task by ORing Bit fields together and adds the task object to an array of ongoing transfers.
The bit field is required, as we have different paths along the request. There is the traditional delegate based path (See NSURLSessionTaskDelegate and friends) which has a large number of mostly optional methods for managing the transfer in detail.
There is also the possiblity of using a completion handler instead. Data is aggregated internally (or just written to a file in case of a downloadTask) and passed as an argument to the completion handler.
The internal options
typedef NS_ENUM(NSInteger, GSURLSessionProperties) {
GSURLSessionStoresDataInMemory = (1 << 0),
GSURLSessionWritesDataToFile = (1 << 1),
GSURLSessionUpdatesDelegate = (1 << 2),
GSURLSessionHasCompletionHandler = (1 << 3),
GSURLSessionHasInputStream = (1 << 4)
};
The pointer to the task instance is now returned to the caller.
Task Resumption
By default, the task is in suspended state. When -[NSURLSessionTask resume] is called for the first time, the session is retained and -[NSURLSession _resumeTask:] is called. -[NSURLSession _resumeTask:] adds the underlying easy handle to the session’s multi handle (curl_multi_add_handle). This in turn triggers a timer setup or reconfiguration (timer_callback).
The timer event handler calls curl_multi_socket_action and checks for task completion.
-[NSURLSession _checkForCompletion] reads the backlog of messages from the multi handle (curl_multi_info_read). If a easy handle is marked as done, the handle is removed from the multi handle (curl_multi_remove_handle), the task removed from the list of active tasks and -[NSURLSessionTask _transferFinishedWithCode:] is called.
For more information on how the libcurl multi socket API works, visit https://curl.se/libcurl/c/libcurl-multi.html
Related fixes and additions:
- NSBlockOperation: Fixed memory leak
- NSHTTPCookie: Fix ‘expires’ date parsing
- NSURLRequest: Implemented assumesHTTP3Capable
- NSURLRequest: Private method to access case-insensitive Header Dictionary
TODO (Priority High to Low):
- Unit Testing on Android
- Unit Tests for TLS (requires HTTPS test server)
- Custom Protocol Loading
- Caching
- Download with Resume Data
- Web Socket Extensions
- Background Downloading via Daemon (on macOS nsurlsessiond
Open Questions:
What is the desired behaviour when libcurl configuration or update functions return configuration errors? The old implementations just throws exceptions everywhere.
This looks great. I'm traveling atm but will try to test this the week after next.
Should we also get the author of the original implementation to take a look at this as well?
@hmelder I think with a change of this volume if you haven't submitted your copyright assignment, then you should get it in as soon as possible.
@hmelder as I had said before just email them and cc me on it. As soon as you do that we should be okay accepting any changes as the FSF process can take a little time. It's electronic now so it's faster than before but since they are chiefly a volunteer organization my policy has always been to proceed as long as the process has been started.
Their email as previously stated is [email protected].
Should we also get the author of the original implementation to take a look at this as well?
@theiostream would you be willing to take a look at my implementation? Not sure if you are still familiar with the code base ^^
but I'm unsure of the NSOperation change (I don't have any practical experience using NSOperation with blocks).
Blocks are copied onto the heap (or ref count is increased if already on heap) and transparently treated as normal Objective-C objects in the Runtime. We add the block to the array of blocks, but AFAIK never remove them after the operation (i.e. calling the block) is completed.
Did a quick check with an NSLog in a -dealloc as well :)
@rupertdaniel could you please test this branch with our Unity project and if you get a chance with our Qt app (Win/Android) as well?
I do not understand why the CI is suddenly failing. It works just fine on Ubuntu, Rocky Linux, and Windows on my Machine (Objc sections are somehow discarded in the CI). Is there an easy way to reproduce a CI build?
I've added more tests and added GSCACertificateFilePath which is a NSUserDefaults key for setting the path to a custom CA blob.
The bug was quite nasty. I have assigned a substring to a variable (headerLine) which is then released shortly after. However, the substring was also added to the arc_tls pool on return. After some time, an already deallocated object was being released on a thread managed by libdispatch causing a crash.
Turns out the latest tools-windows-msvc release build does not ship libcurl. I started a manual CI run which uses the master branch.
Here the test results on Windows 11 ARM64
OsName : Microsoft Windows 11 Pro
OsType : WINNT
OsOperatingSystemSKU : 48
OsVersion : 10.0.22631
WindowsBuildLabEx : 22621.1.arm64fre.ni_release.220506-1250
WindowsCurrentVersion : 6.3
OSDisplayVersion : 23H2
hugo@YellowBox MSYS /c/Users/hugo/Documents/libs-base/Tests/base/NSURLSession
$ gnustep-tests .
--- Running tests in . ---
84 Passed tests
All OK!
@gcasa, you are also invited to review the PR, if you'd like to :)
Naturally, of course, I prefer to leave it up to @rfm as he has more knowledge of this code than I do.
I've implemented a custom redirection system for non-convenience tasks. See https://developer.apple.com/documentation/foundation/nsurlsessiontaskdelegate/1411626-urlsession
I’ve rebased and squashed all NSURSession-related commits. @hmelder would be great if you can amend a change log entry.