Skip to content

Prevent reconnection crash#294

Merged
daltoniam merged 2 commits into
daltoniam:masterfrom
wordpress-mobile:prevent-reconnection-crash
Dec 30, 2016
Merged

Prevent reconnection crash#294
daltoniam merged 2 commits into
daltoniam:masterfrom
wordpress-mobile:prevent-reconnection-crash

Conversation

@koke

@koke koke commented Dec 27, 2016

Copy link
Copy Markdown
Contributor

There are a couple cases where a socket would create a new connection without closing an existing one.

  1. Between the time when the socket opens the streams until the TCP handshake ends, it would be both !connected and !isConnecting, so a second connect would go through creating a new pair of streams without disconnecting the first pair.
  2. Since disconnect without a timeout only sends a close message to the server, if we tried to connect the socket again before the connection was closed it would not disconnect the previous streams.

In any of those cases we end up with a NSInputStream that still has the WebSocket as a delegate, but it's no longer referenced in inputStream. If the WebSocket is deallocated, it wouldn't clean up the delegate on that stream, and if it received any data, it would try to forward it to a deallocated object, which would crash.

This should fix #272

koke added 2 commits December 27, 2016 15:56
The existing code only prevented a second connection to be created while
createHTTPRequest was being called (setting up the HTTP request and the
streams). However, between the time when the socket opens the streams until the
TCP handshake ends, it would be both `!conneted` and `!isConnecting`, which
wouldn't prevent a second `connect`.
Since `disconnect` without a timeout only sends a close message to the server,
if we tried to `connect` the socket again before the connection was closed it
would not clean up the streams properly, which could result in a crash.

See daltoniam#272
@daltoniam

Copy link
Copy Markdown
Owner

Ah very good. Thank you for the great write up and PR!

@daltoniam daltoniam merged commit 3ba1963 into daltoniam:master Dec 30, 2016
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

Successfully merging this pull request may close these issues.

Establishing new connections connections causes app to crash over time

2 participants