Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Starscream hangs when NSStream closes due to connectivity issues #164

Closed
dkharrat opened this issue Jan 27, 2016 · 7 comments
Closed

Starscream hangs when NSStream closes due to connectivity issues #164

dkharrat opened this issue Jan 27, 2016 · 7 comments

Comments

@dkharrat
Copy link

I was testing the case when my websocket server is down, and noticed that Starscream hangs when it receives the NSStreamEvent.ErrorOccurred code after initialization in initStreamsWithData.

Specifically, it hangs on the following line in disconnectStream():

writeQueue.waitUntilAllOperationsAreFinished()

I tracked it down, and it seems that it's due to the following operation:

    writeQueue.addOperationWithBlock {
        while !outStream.hasSpaceAvailable {
            usleep(100) //wait until the socket is ready
        }
        outStream.write(bytes, maxLength: data.length)
    }

Essentially, there's an infinite loop in the while loop waiting for the stream to be available, as outStream.hasSpaceAvailable is never true. This causes a deadlock in the disconnect waiting for writeQueue to be empty. I was able to resolve it as follows:

    writeQueue.addOperationWithBlock {
        while outStream.streamError == nil && !outStream.hasSpaceAvailable {
            usleep(100) //wait until the socket is ready
        }
        outStream.write(bytes, maxLength: data.length)
    }

Is this the right fix? If so, I can open a pull request with the fix.

@nuclearace
Copy link
Contributor

I noticed this when debugging #158

@daltoniam
Copy link
Owner

This is probably related to #161. I have a fix I am testing for both #158 and #161 (and subsequently this one), this is similar to the fix I have. Hopefully can get that pushed up later tonight, just haven't had a free moment yet 😞

daltoniam added a commit that referenced this issue Jan 28, 2016
@daltoniam
Copy link
Owner

I just checked in a fix into master. Let me know if that seems to resolve it and I will do another release.

@dkharrat
Copy link
Author

Unfortunately, the change does not fix it. The change now actually introduces a deadlock and it ends up still hanging in the same place. The reason is, on timeout self.disconnectStream(error) is called within the queue operation block, but disconnectStream() also waits for the operations to finish before proceeding, which will never happen since the ongoing operation is also waiting on disconnectStream() to return.

Do you need to call disconnectStream() within the operation block on timeout/error? I propose not calling it all. NSStream would be calling the NSStreamDelegate anyways with the error code, and the delegate implementation will call the disconnectStream() upon an error code. This is essentially what my fix above does.

The bug is easily reproducible by simply having Starscream connect to a non-running WebSocket server.

daltoniam added a commit that referenced this issue Jan 29, 2016
@daltoniam
Copy link
Owner

Ah, good point totally overlooked that. You are correct the delegate will call it, I also overlooked that. I check in another fix that I believe should handle it now. Thank you for the feedback.

@Tarpsvo
Copy link

Tarpsvo commented Feb 1, 2016

Had the same issue and lost quite a bit of time debugging. Works fine after applying your latest fixes. Cheers!

@daltoniam
Copy link
Owner

1.1.2 released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants