14

I use the SocketRocket library for Objective-C to connect to a websocket:

-(void)open {

if( self.webSocket ) {
    [self.webSocket close];
    self.webSocket.delegate = nil;
}

self.webSocket = [[SRWebSocket alloc] initWithURLRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"ws://192.168.0.254:5864"] cachePolicy:NSURLRequestUseProtocolCachePolicy timeoutInterval:20]];
self.webSocket.delegate = self;
[self.webSocket open];
}

Opening the connection works totally fine. The delegate is called after the connection was established.

-(void)webSocketDidOpen:(SRWebSocket *)webSocket {

NSLog(@"WebSocket is open");

}

But when I want to close the connection, nothing happens.

-(void)close {

if( !self.webSocket )
    return;

[self.webSocket close];
self.webSocket.delegate = nil;

}

The delegate for successfully closing the connection is not called. Can anyone tell me why this happens?

Thank you for reading my question.

Philipp Otto
  • 4,061
  • 2
  • 32
  • 49

3 Answers3

4

I figured out that the delegate is never called, because the websocket is never really closed. The closing of the websocket in the SRWebSocket happens in the method pumpWriting like this:

if (_closeWhenFinishedWriting && 
    _outputBuffer.length - _outputBufferOffset == 0 && 
    (_inputStream.streamStatus != NSStreamStatusNotOpen &&
     _inputStream.streamStatus != NSStreamStatusClosed) &&
    !_sentClose) {
    _sentClose = YES;

    [_outputStream close];
    [_inputStream close];

    if (!_failed) {
        dispatch_async(_callbackQueue, ^{
            if ([self.delegate respondsToSelector:@selector(webSocket:didCloseWithCode:reason:wasClean:)]) {
                [self.delegate webSocket:self didCloseWithCode:_closeCode reason:_closeReason wasClean:YES];
            }
        });
    }

    _selfRetain = nil;

    NSLog(@" Is really closed and released ");
}
else {

    NSLog(@" Is NOT closed and released ");
}

All streams and an object to retain the websocket are closed or deleted there. As long as they are still open, the socket won´t be closed appropriately. But the closing never happened in my program, because when I tried to close the websocket, _closeWhenFinishedWriting was always NO.

This boolean is only set once in the disconnect method.

- (void)_disconnect;
{

assert(dispatch_get_current_queue() == _workQueue);
SRFastLog(@"Trying to disconnect");
_closeWhenFinishedWriting = YES;
[self _pumpWriting];

}

But when calling the closeWithCode method in SRWebSocket, disconnect is only called in one case and that is, if the websocket is in the connecting state. 

BOOL wasConnecting = self.readyState == SR_CONNECTING;

SRFastLog(@"Closing with code %d reason %@", code, reason);
dispatch_async(_workQueue, ^{

    if (wasConnecting) {
        [self _disconnect];
        return;
    }

This means, if the socket is in another state, the websocket will never really close. One workaround is to always call the disconnect method. At least it worked for me and everything seems to be alright.

If anyone has an idea, why SRWebSocket is implemented like that, please leave a comment for this answer and help me out.

Philipp Otto
  • 4,061
  • 2
  • 32
  • 49
4

I think this is a bug.
When calling close, the server echo's back the 'close' message.
It is received by SRWebSocket, however the _selfRetain is never set to nil, and the socket remains open (the streams are not closed) and we have a memory leak.
I have checked and observed this in the test chat app as well.
I made the following change:

-(BOOL)_innerPumpScanner {    
    BOOL didWork = NO;

    if (self.readyState >= SR_CLOSING) {
        [self _disconnect];  // <--- Added call to disconnect which releases _selfRetain
        return didWork;
    }

Now the socket closes, the instance is released, and the memory leak is gone.
The only thing that I am not sure of is if the delegate should be called when closing in this way. Will look into this.

emp
  • 3,350
  • 1
  • 22
  • 19
  • Yeah you´re right. As I mentioned in my answer, the _disconnect has to be called to set the BOOL _closeWhenFinishedWriting to YES, so that the _selfRetain can be set to nil appropriately – Philipp Otto Jun 19 '13 at 12:42
  • Correct. There is the case when a server dies, the delegate won't be notified, and will just sit there. I made changes so the delegate gets notified. I created a [pull request](https://github.com/square/SocketRocket/pull/106) if anyone wants to see the changes I made. – emp Aug 06 '13 at 21:05
  • This issue has been address on GitHub, although the fix has not been merged. https://github.com/mediumbear/SocketRocket/commit/08afedbeb991edfda13aa675b5720c53ffaf7ef8 – Jeremy White Oct 28 '13 at 21:03
  • Yup - quite a lot of forks from that commit, from what I can see. – Scott Corscadden Oct 31 '13 at 02:29
  • I am always getting this error and Test to Speech getting Audio interruption, WebSocket closed with reason[1000]: Manually terminating socket connection Any suggestions? – Anilkumar iOS - ReactNative Aug 17 '17 at 08:54
2

Once an endpoint has both sent and received a Close control frame, that endpoint SHOULD Close the WebSocket Connection as defined in Section 7.1.1. (RFC 6455 7.1.2)

The SRWebSocket instance doesn't _disconnect here because that would close the TCP connection to the server before the client has received a Close control frame in response. In fact, _disconnecting here will tear down the TCP socket before the client can even send its own Close frame to the server, because _disconnect ultimately calls _pumpWriting before closeWithCode: can. The server will probably respond gracefully enough, but it's nonconforming, and you won't be able to send situation-unique close codes while things are set up this way.

This is properly dealt with in handleCloseWithData:

if (self.readyState == SR_OPEN) {
    [self closeWithCode:1000 reason:nil];
}
dispatch_async(_workQueue, ^{
    [self _disconnect];
});

This block handles Close requests initiated by both the client and the server. If the server sends the first Close frame, the method runs as per the sequence you laid out, ultimately ending up in _pumpWriting via closeWithCode:, where the client will respond with its own Close frame. It then goes on to tear down the connection with that _disconnect.

When the client sends the frame first, closeWithCode: runs once without closing the TCP connection because _closeWhenFinishedWriting is still false. This allows the server time to respond with its own Close frame, which would normally result in running closeWithCode: again, but for the following block at the top of that method:

if (self.readyState == SR_CLOSING || self.readyState == SR_CLOSED) {
    return;
}

Because the readyState is changed on the first iteration of closeWithCode:, this time it simply won't run.

emp's bug fix is necessary to make this work as intended, however: otherwise the Close frame from the server doesn't do anything. The connection will still end, but dirtily, because the server (having both sent and received its frames) will break down the socket on its end, and the client will respond with an NSStreamEventEndEncountered:, which is normally reserved for stream errors caused by sudden losses of connectivity. A better approach would be to determine why the frame never gets out of _innerPumpScanner to handleCloseWIthData:. Another issue to keep in mind is that by default, close just calls closeWithCode: with an RFC-nonconforming code of -1. This threw errors on my server until I changed it to send one of the accepted values.

All that said: your delegate method doesn't work because you're unsetting the delegate right after you call close. Everything in close is inside an async block; there won't be a delegate left to call by the time you invoke didCloseWithCode: regardless of what else you do here.

Community
  • 1
  • 1
James McMath
  • 21
  • 1