twitter-archive / CocoaSPDY

SPDY for iOS and OS X
Apache License 2.0
2.39k stars 233 forks source link

Support for server push streams #136

Closed kgoodier closed 8 years ago

kgoodier commented 8 years ago

Summary

This implementation of push streams keeps the public api relatively unchanged. Pushed streams are cached in-memory inside CocoaSPDY until either the response finishes or the associated stream finishes, at which point the pushed streams are discarded. While the pushed stream is alive and in the cache, the app may create a new request, just like any other; if CocoaSPDY sees this request matches a pushed request, it will hook the two up and issue all the usual NSURLProtocolClient delegate callbacks.

The app may register for the SPDYPushRequestReceivedNotification NSNotification, which will be sent for every push stream received. This allows for "early detection" of pushed requests. Alternately, it is up to the app to parse the response of the original request, discover related resources, and issue requests for them, all before the pushed streams are removed from the cache.

The app can tell if a response was pushed or not by looking at the SPDYMetadata for the response -- the streamId will be an even number (2, 4, 6, etc) for pushed streams.

Implementation details

New methods in SPDYStream for creating and starting pushed streams. Clones the associated stream, particularly its delegate and original request. Once new headers come in, creates a new NSURLRequest based off original but with new values as appropriate.

The app is alerted to new pushed requests via a global NSNotification that it can optionally subscribe to. It is the app's responsibility to filter these for relevant origins / urls and decide whether to create a new NSURLConnection or NSURLSession-based request to hook up to the pushed stream.

The push stream manager is a per-origin cache of currently open push streams. It serves as the URLProtocolClient in order to buffer lifetime events like the response, data chunks, and finalization. New requests coming from the app are checked against this cache, matching the canonical URL. If a match is found, the stream is removed from the push cache and appropriate URLProtocolClient catch-up callbacks are made.

The cache also maintains a mapping of associated stream to push streams. If the original (associated) stream is cancelled, all unclaimed, in-progress push streams are cancelled. The only way to cancel a single in-progress push stream is to start a new NSURLRequest to hook it up to the push stream, then cancel it. We may want to improve this.

Refactor stream cancel/close code. Error cases like receiving headers with an invalid status code should send a reset frame with a protocol error indicated, but previously the code was only closing the stream locally. Those have been fixed, and the steps needed to properly end a stream have been cleaned up.

The session test code has been refactored into a base class, also in prep for server push changes coming.

Remaining work

Allow canceling push stream from NSNotification handler. Private NSURLCache for unclaimed push streams, for life of session. Mock integration tests for end-to-end experience. SPDYPushStreamManager request equivalency check.

Work inspired by @layerhq's push stream pull request. Big thanks to @blakewatters and @chipxsd. Resolves issue https://github.com/twitter/CocoaSPDY/issues/1

NSProgrammer commented 8 years ago
Pushed streams are cached in-memory inside CocoaSPDY
until either the response finishes or the associated stream finishes,
at which point the pushed streams are discarded.

Does this mean if a pushed response is not immediately accessed the original response will finish and the pushed response will be discarded? Seems pretty racy. Why not keep the pushed response in a cache that clears as it encounters pressure?

kgoodier commented 8 years ago

@NSProgrammer you are correct (though to clarify, pushed responses live until they are finished, even if the original response finishes first). There are 2 ways to mitigate the race, but it will never go away entirely (which is true for any solution that CocoaSPDY could provide, really):

  1. Subscribe to the NSNotification and quickly filter for interesting NSURLRequests and start new requests for them before returning.
  2. Delay returning from the final delegate call (connectionDidFinishLoading, connection:didFailWithError:, URLSession:task:didCompleteWithError:, etc) -- create the new NSURLRequest for the relevant pushed stream before returning.

As apps start actually using the pushed streams, we may want to augment this strategy with an internal CocoaSPDY cache, or find some other solution. I'm not claiming that what is in this pull request is sufficient & robust -- just that it is good enough to get this out the door, with the expectation of future iterations.

plivesey commented 8 years ago

Thanks for starting to merge this into master. This looks great.

NSProgrammer commented 8 years ago

Just 1 nitpick: +1

kgoodier commented 8 years ago

Tested push functionality against a simple test server running https://github.com/indutny/node-spdy. Behavior looks good. Also tested against a Netty-based server a while ago in a less contrived setup. In addition, tested non-push-related functionality in a real-world setup and all looks good. From my view, this pull request is ready to merge, and I'll do so tomorrow morning unless anyone else speaks up.