search

pusher / pusher-websocket-swift

Pusher Channels websocket library for Swift
https://pusher.com/channels
MIT License
273 stars 166 forks source link

Update PusherSwift to use Starscream 4.x.x #277

Closed JonathanDowning closed 4 years ago

JonathanDowning commented 4 years ago

Steps to reproduce

N/A

Expected behavior

N/A

Actual behavior

N/A

Any improvements you suggest

I noticed that Starscream 3.1.1 has a couple of possible dangling pointer bugs in it's compression code, which could well be causing crashes in our app (although I've no direct evidence of this) In researching the Starscream codebase I noticed it's now on 4.0.2 and looks much improved.

Are there plans to use this in Pusher soon?

wangdu1005 commented 4 years ago

Why we use PureSwift have to use Starscream in iOS Swift? I never read any Docs of installation have to use it (Starscream). Can someone help me to figure out?

danielrbrowne commented 4 years ago

@wangdu1005 The PusherSwift library itself depends on Starscream (this is used for creating the WebSocket connections with the Pusher platform). So it's not really "used" directly by any developers that use the PusherSwift library.

If you follow the steps in the Installation section of the README, you'll see how to integrate the PusherSwift library within your own iOS or macOS application, using Cocoapods, Carthage or Swift Package Manager as your chosen dependency manager.

chickdan commented 4 years ago

We also have a handful of crashes that are traced back to Starscream. EXC_BAD_ACCESS0x89cc (-0x102e64000 + 4343646668) Attempted to dereference garbage pointer 0xee8516020.

danielrbrowne commented 4 years ago

To update interested parties:

I have recently joined Pusher and I will be the primary maintainer of this SDK. I am currently working on a branch to move from Starscream 3.1.1 -> 4.x.x . Currently, I have run into an issue related to how we're mocking their WebSocket object for our unit tests, which is blocking my progress as I'd like to avoid introducing websocket connections over a live network as part of our unit testing. I've filed an issue here which, if resolved should unblock my progress: https://github.com/daltoniam/Starscream/issues/811

JonathanDowning commented 4 years ago

Thanks for the update @danielrbrowne ! :)

JonathanDowning commented 4 years ago

@danielrbrowne I've opened this PR to help get this going https://github.com/daltoniam/Starscream/pull/812

danielrbrowne commented 4 years ago

Considering that I am currently not getting any response from the Starscream repo maintainer, I am looking into URLSessionWebSocketTask as a potential plan B.

Even if it is feature compatible with Starscream v4.x, this of course would have its own separate considerations since it would increase the minimum deployment target versions of our SDK:

Given these are all the current release versions, this change may need to wait until iOS 14.0 and macOS 11.0 are released this autumn (if this native solution to Web Sockets is used).