Implement SPDY Push

[blakewatters]

This changeset introduces support for SPDY Push by utilizing a new delegate on SPDYSession. Closes #1

This is a focused subset of the larger changes that we have been maintaining in the @layerhq branch. This isolates the changes necessary to support SPDY Push.

Utilizing push requires registering a delegate for the SPDYSessionManager so that you can attach a delegate to each SPDYSession as it is dispatched by CocoaSPDY. You must also issue a hanging GET request so that the server has a SPDY stream that it can push responses back against. We’re using it successfully with an Erlang server utilizing the Cowboy server.

[kgoodier]

Thanks for splitting this out and cleaning things up. Just so we're on the same page, here's my read of the changes at a high level. Is this right?

The app is required to track sessions coming & going, and must attach a delegate to each session in order to be notified of incoming push responses. The pushed data is buffered in the stream until done, at which point the app is notified all at once. Is there a way for the app to cancel a pushed response? Is there any way for the app to know which original request the pushed responses are associated with?

Thanks Blake.

[blakewatters]

Your assessment of the implementation is spot on. As to your questions:

You can cancel the pushes by canceling the original NSURLRequest that established the SPDY stream that the responses are being pushed in response to. In our implementation we have openPushChannel and closePushChannel methods that hold onto a dedicated NSURLSession that we use for initiating the push request. When we want to disable SPDY pushes we simply tell it to invalidateAndCancel. The other quirk with this stuff is that you have to boost the NSURLRequest timeout way up. Here’s a quick snippet showing our usage:

- (void)openPushChannel
{
    NSURLSessionConfiguration *pushConfiguration = [self.sessionConfiguration copy];
    pushConfiguration.allowsCellularAccess = YES;
    NSTimeInterval timeoutInterval = 60 * 60 * 24; // 24 hours
    pushConfiguration.timeoutIntervalForRequest = timeoutInterval;
    pushConfiguration.timeoutIntervalForResource = timeoutInterval;
    pushConfiguration.requestCachePolicy = NSURLRequestReloadIgnoringLocalAndRemoteCacheData;
    NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:@"push" relativeToURL:self.baseURL]];
    request.allHTTPHeaderFields = self.URLSession.configuration.HTTPAdditionalHeaders; // Radar 17809816 Workaround
    request.timeoutInterval = timeoutInterval; // Radar 17809816 Workaround
    LYRLogVerbose(@"opening a push channel on endpoint: %@", request.URL);
    NSURLSession *pushSession = [NSURLSession sessionWithConfiguration:pushConfiguration];
    [[pushSession dataTaskWithRequest:request completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) {
        if (error) {
            if (!([error.domain isEqualToString:SPDYStreamErrorDomain] && (error.code == SPDYStreamCancel || error.code == SPDYSocketTransportError)) &&
                !([error.domain isEqualToString:NSURLErrorDomain] && (error.code == NSURLErrorCancelled))) {
                LYRLogError(@"received a response when opening a push channel: %@ %@", response, error);
            }

            if (self.isConnected) {
                LYRLogDebug(@"Reopening SPDY push channel");
                [self openPushChannel];
            } else {
                LYRLogDebug(@"Declining to reopen SPDY push channel: transport is not currently connected.");
            }
        }
    }] resume];
    [pushSession finishTasksAndInvalidate];
    self.pushSession = pushSession;
}

- (void)closePushChannel
{
    [self.pushSession invalidateAndCancel];
    self.pushSession = nil;
}

- (BOOL)isPushChannelOpen
{
    return self.pushSession != nil;
}

We could presumably figure out the request that the response was pushed against. We haven’t needed that info in our implementation so we haven’t done the homework. Presumably we’d just need to do the homework around tracing the SPDY stream ID back to the request and then deliver it with the callback. I can try to knock this out if you’d like.

[blakewatters]

Will fix the change around LOCAL_MAX_CONCURRENT_STREAMS

[kgoodier]

Thanks Blake! I've had more time to digest this and talk it over with @goaway, and this looks like a great starting point that we definitely want to use. I'm going to pull it into a local branch and iterate on it and see where it goes. There are a few things that I think will have to change, as we have divergent requirements from you. I'd like to ensure that they will meet our needs, your needs, and the needs of anyone using CocoaSPDY in the future.

The spirit of SPDY push requests, per the spec, is that of resources a client will likely request in the near future as part of processing a regular response. That's why there must be an associated stream id. Given this, it would make sense to notify the app of new push responses in a manner that ties them to the original request, such as a delegate that is supplied in the NSURLRequest, rather than being tied to the SPDY session.

• The push responses should be immediately cancelable by the app, or cancelable at any time thereafter, without affecting the original request.
• They should chunk the data out to the app in case these are large transfers.
• They should also potentially be inserted into a cache by CocoaSPDY, if configured / allowed by the app.

Your model is more of a permanent channel for the server to communicate back with the client, if I understand correctly. While not true to the philosophy, I understand the need. I think the model I proposed above would still work for this -- your long-lived GET would receive the notifications about new push responses. Seems like a fairly minor change for you?

The session management changes may have to be substantially altered. I think... need to dive deeper. There's a major revision of the object ownership model under consideration (see https://github.com/twitter/CocoaSPDY/compare/new_dispatch) that gives a lot of good benefits -- better dispatching, better stream load balancing, more flexibility with creating / killing tcp connections, etc. But it's going to conflict. I don't believe the changes you made are necessary if one doesn't need to be aware of sessions coming & going via a new delegate, but I may have misread the code. Moving that notification delegate for new push responses into the NSURLRequest is better anyway, as it allows for request-by-request decision on whether pushed responses are processed or not, and they can be better localized to the code that made the original requests and presumably knows what future resources it will want.

Let me know what you think, or especially if I've misunderstood anything. Thanks a lot for taking the time to help out!

[blakewatters]

All your enhancements to the push support make good sense and evolve the implementation from our specific use case needs into a more robust implementation. We’re currently only using the SPDY push channel to transmit very small Thrift objects in response to real time messaging events so chunking, streaming, and caching never hit our requirements. I don’t see any issues for us in the changes you outlined on push. Looking forward to getting behind an aligned implementation.

I read through the changes on the new_dispatch branch and for the large part it looks like the changes would be transparent for us with one major exception: localManagerForOrigin: is still using thread local storage via [NSThread currentThread].threadDictionary for looking up the origins and thus the streams. This makes it impossible to grab the list of streams from the main thread and thus means my test suite will be wrecked and I won’t be able to spin down the connection on demand.

On a tangentially related note, I do have another branch that ports CocoaSPDY to using Grand Central Dispatch laying around. The driver for this was that I could access the thread execution context at will and not have to rely on NSURLProtocol to wake me up inside the NSURLConnectionLoader thread in order to get scheduled on the proper run loop. It allowed me to drop the use of NSTimer, NSRunLoop, and various incantations of performSelector across the library at the expense of passing around an object wrapping a dispatch queue. I generally kind of hate the thread isolation box that we are currently stuck in, though the CFRunLoopPerformBlock trickery has been an acceptable workaround (although there’s an annoying bootstrap problem with obtaining the run loop reference the first time). The GCD code was running pretty stable and I could break it out in a standalone patch for review if there’s interest. It does carry with it a minimum version target of iOS 7 because it relies on CFReadStreamSetDispatchQueue and CFWriteStreamSetDispatchQueue to schedule the streams.

[kgoodier]

I've been working on this the past few weeks and have something nearly done. I squashed it into a single commit to make it easy to review, see kgoodier@46e8cbe. The evolution was basically:

1. Start with this pull request
2. Get it onto "develop" instead of "main".
3. Undo session manager and interface changes.
4. Implement new interfaces (see SPDYProtocol.h) incrementally
5. Inspect code for issues.
6. Write unit tests. Fix bugs.
7. Goto 4 until all interfaces and enough unit tests done.

I started with the tests you guys implemented in your branch, but refactored them substantially and added a lot more. I think the comments and interfaces in SPDYProtocol.h should be sufficient to show how to actually use these new push requests, but would appreciate feedback. It's different from what you had but shouldn't be hard to switch to.

A few small TODO's remain. I also need to merge in the upcoming changes in 'develop' for 'new_dispatch'.

[blakewatters]

@kgoodier did this work get abandoned? I’m looking at brings my fork inline with the develop branch and it looks like there’s still no push support

[kgoodier]

Hi @blakewatters, nope, this is still in progress, just got de-prioritized for a bit. I ran into some subtle issues about how to expose request/response metadata, and that led to some changes to the push API presented here. It got simplified a lot so the app doesn't have to do much at all or implement any new interfaces.

For your use case of a long pull followed by notifications of new pushed streams, it should work like:

1. register for an NSURLNotification for incoming push streams
2. issue long pull
3. server sends back push streams associated with the long pull
4. spdy stores these in a temporary "in-progress" cache and issues a notification
5. when app gets notification, issue a new NSURLRequest using the url for the pushed stream. this will get hooked up to the already-in-progress push stream, and proceeds like any other standard NSURL-based http request.

I'm working on the final integration tests to make sure everything actually works, but the bulk of the coding is done except for the NSNotification stuff. I won't get much done next week, but expect to focus 100% on finishing this up the week after.

We're also intending to get the current develop branched merged into master and make another release. It's been a while and a lot of good things have gone in. That will happen prior to push getting integrated.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}