search

twitter-archive / CocoaSPDY

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

Thread Safety Concerns #48

Closed blakewatters closed 10 years ago

blakewatters commented 10 years ago

I wanted to open up a discussion about the approach to threading utilized in the library. CocoaSPDY is presently totally reliant on the socket being interacted with only through the NSURLConnectionLoader thread to guarantee its safety.

This has presented a number of challenges:

  1. In my test suite it requires acrobatics to gain control over the sockets and ensure that connections are torn down between test cases. Once I solved the issue of getting access to the sessions we face random crashes when socket activity occurs while a test is trying to shut down the session/socket from the main thread.
  2. Outside of a testing context we also have a desire to control the sockets independent of the NSURLProtocol interface.

In general I find GCD much easier to work with and reason about than runloops — particularly on threads I can’t access at will.

I have a patch set in development that introduced a GCD queue into the SPDYSocket class to ensure thread safety. This eliminates the need for CHECK_THREAD_SAFETY macros by providing strong guarantees of thread safety. Is this work you would consider merging?

blakewatters commented 10 years ago

Work rewiring the internals to use GCD and a dedicated NSThread + NSRunLoop is on the @layerhq fork: https://github.com/layerhq/CocoaSPDY/pull/2

blakewatters commented 10 years ago

This problem is actually easily solved by utilizing CFRunLoopPerformBlock to ensure closing of the SPDY socket takes place on the NSURLConnection thread.

We also have an implementation of CocoaSPDY that runs its socket and streams on a dispatch queue instead of run loops but it has never felt as solid as the runloop approach.

kgoodier commented 10 years ago

Do you have an example of the runloop implementation that uses CFRunLoopPerformBlock? Also, do you have concrete examples of why the GCD approach is less solid, or is it just a gut feeling?

kgoodier commented 10 years ago

Ah, I see it in SPDYSession.m's close method in https://github.com/twitter/CocoaSPDY/pull/59.

blakewatters commented 10 years ago

The branch that uses GCD exhibits a strange stalling behavior that we have not been able to pin down. The changes also required trampolining from basically every public method back onto the dispatch queue in order to guarantee thread safety so its a considerably heavier change than the perform block change.