square/SocketRocket — pull request #67 by michaelpetrov (2013-01-06 00:43:58+00:00)
Title: Made closeWithCode:reason: safer by moving readyState change to work queue

SocketRocket has been working great in production for a long time and is generally very stable, thank you! However we found a race condition that at times triggers an assert under the following conditions:
1. The websocket is in the process of connecting/header parsing/handshake (happening on the internal-work-queue)
2. A disconnect happens from the server side, triggering some delegates that as a precaution will close the socket using [socket close]
3. the [socket close] call gets called on the main thread (or any other non internal-work-queue thread)

On of two cases occasionally happen:
- self.readyState gets set to SR_CLOSING on the main thread, then the work queue handshake code tries to set it to SR_OPEN causing an assert to fire (usually this would be in _HTTPHeadersDidFinish)
- Setting self.readyState to SR_CLOSING fires the assert, implying that something else has moved the readyState to SR_CLOSED (or SR_CLOSING) in a different state

The assert in question is this:
```
- (void)setReadyState:(SRReadyState)aReadyState;
{
[self willChangeValueForKey:@"readyState"];
assert(aReadyState > _readyState);
_readyState = aReadyState;
[self didChangeValueForKey:@"readyState"];
}
```

I moved the code that checks and manipulates the readyState into the work queue and it seems to fix our bug. However, I would welcome feedback as to why this might be unsafe. Was there a reason to set it to SR_CLOSING before the work queue operation block? If there is a reason, perhaps there should be some synchronize blocks around any access to the readyState?