Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Support for server push streams #136

Merged
merged 9 commits into from
Oct 30, 2015

Conversation

kgoodier
Copy link
Contributor

@kgoodier kgoodier commented Oct 6, 2015

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 #1

Split the merging of stream headers from the processing of the response.
Modify SPDYStream to support initializing as a push stream, and
processing the "request" side of that (essentially the SYN_STREAM).
Duplicate the original request but replace the URL.

Basic push tests are included here, but more are coming later.

The ability for the app to actually hook up to a pushed stream is also
coming later.
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 manager also maintains a mapping of associated streams 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.
Post an NSNotification when a push request is received. This will allow
the app to use the provided NSURLRequest and filter for ones of
interest, potentially starting a new load for that request. CocoaSPDY
will hook up that request to start loading to the pushed request using
SPDYPushStreamManager.
@kgoodier kgoodier mentioned this pull request Oct 6, 2015
@NSProgrammer
Copy link
Collaborator

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
Copy link
Contributor Author

kgoodier commented Oct 7, 2015

@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
Copy link

plivesey commented Oct 7, 2015

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

@NSProgrammer
Copy link
Collaborator

Just 1 nitpick: +1

@kgoodier
Copy link
Contributor Author

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.

kgoodier added a commit that referenced this pull request Oct 30, 2015
@kgoodier kgoodier merged commit acc0123 into twitter-archive:develop Oct 30, 2015
@kgoodier kgoodier deleted the feature/spdy-push-develop branch October 30, 2015 18:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants