Starscream
Starscream copied to clipboard
[CRASH] CoreFoundation _inputStreamCallbackFunc
I did a little bit of investigation into this issue ( #676 ) since I encountered it. If the application releases the WebSocket instance before the disconnect is completed the FoundationTransport
is released but it has not yet been stopped. So messages (from 'CFReadStream'/'CFWriteStream') to their delegates will still get fired and will result in this crash.
I'm not sure what the right solution is to the problem; however, deinit
in FoundationTransport
to do the right cleanup or ensuring that engine doesn't release the transport until it is finished stopping (since the stop is currently async, and only weakly holds onto self in the completion handler).
Otherwise the client needs to wait until the socket has been completely closed before releasing its instance (which would need docs -- but I'd much prefer it doesn't crash if you release the webSocket object).
A simple test for this problem:
Connect a websocket and then...
webSocket.delegate = nil
webSocket.disconnect()
webSocket = nil
I fixed it with these (#777) , thanks
deinit {
if #available(iOS 12, *) { } else {
if let mirror = Mirror(reflecting: self).superclassMirror {
if let inputStream = mirror.descendant("engine", "transport", "inputStream") as? Stream {
inputStream.delegate = nil
}
if let outputStream = mirror.descendant("engine", "transport", "outputStream") as? Stream {
outputStream.delegate = nil
}
}
}
}
@shalyf that implementation seems very fragile. What's the reason behind all those extra checks?
Given that inputStream
and outputStream
are both private, and the implementation of disconnect()
sets their delegates to nil
without any check like you've done, I think it's safe to take the simpler approach I implemented in my #777 solution. I'm open to hearing other opinions though.
@jaltreuter I have MyWebSocket inherited from WebSocket and implement deinit to fix this crash. My code is based on your solution, use Mirror to access private property, because I don't want to modify the framework. Before the author updates, this is my temporary code.
@shalyf thanks for the idea about a temporary solution, based on @jaltreuter PR 👍
Just updated to 4.0.4 (which seems to have the #777 fix) as part of updating to a recent apollo-ios release and have started seeing this crash reported from our app. @jaltreuter can you confirm your fix resolved the issue for you? Just wondering if this issue still exists in the Starscream library or if its more likely an issue with how its being used within apollo-ios. Thanks in advance!
@philfi yes I can confirm that's the case. I updated to 4.0.4 several months ago and haven't seen this crash since then.
@jaltreuter Thanks for confirming. Will continue to dig into it on the apollo-ios side. Fwiw the issue does appear related to the migration from Starscream 3.1.1 to 4.0.4 as the crashes (reportedly) don't occur in apollo-ios releases that use Starscream 3.1.1 and the issue appears in the release that updated to 4.0.4.