davidstump / SwiftPhoenixClient

Connect your Phoenix and iOS applications through WebSockets!
MIT License
507 stars 147 forks source link

race condition in close/join events with v1 serialiser #202

Closed ruslandoga closed 2 years ago

ruslandoga commented 2 years ago

👋

What happens

We are encountering a race condition where phx_close events for channel instances of phx_leaved join_refs are closing channel instances of newer (not phx_leaved) join_refs.

Example logs:

// [SOCKET] SwiftPhoenixClient: push, Sending
{
  "topic": "profile:0000017c-14c7-9745-0242-ac1100020000",
  "ref": "6",
  "event": "phx_join",
  "payload": {},
  "join_ref": "6"
}

// [SOCKET] SwiftPhoenixClient: receive , 
{
  "event": "phx_reply",
  "payload": {
    "response": {
      "profile": {/* ... */}
    },
    "status": "ok"
  },
  "ref": "6",
  "topic": "profile:0000017c-14c7-9745-0242-ac1100020000"
}

// [SOCKET] SwiftPhoenixClient: push, Sending 
{
  "payload": {},
  "join_ref": "6",
  "ref": "15",
  "event": "phx_leave",
  "topic": "profile:0000017c-14c7-9745-0242-ac1100020000"
}

// [SOCKET] SwiftPhoenixClient: push, Sending 
{
  "ref": "21",
  "event": "phx_join",
  "payload": {},
  "join_ref": "21",
  "topic": "profile:0000017c-14c7-9745-0242-ac1100020000"
}

// [SOCKET] SwiftPhoenixClient: receive ,
{
  "event": "phx_close",
  "payload": {},
  // https://github.com/phoenixframework/phoenix/blob/a3a470e6bc4ecefbfac783f465e0fa130f672d4c/lib/phoenix/socket.ex#L690-L693
  "ref": "6", // this ref = join_ref for phx_close events, but it doesn't seem to be used in Channel.isMember checks
  "topic": "profile:0000017c-14c7-9745-0242-ac1100020000"
}

// new channel is being closed
// [SOCKET] SwiftPhoenixClient: channel, close topic: profile:0000017c-14c7-9745-0242-ac1100020000 joinRef: 21

// [SOCKET] SwiftPhoenixClient: receive ,
{
  "event": "phx_reply",
  "payload": {
    "response": {
      "profile": {/* ... */},
      "status": "ok"
    }
  },
  "ref": "21",
  "topic": "profile:0000017c-14c7-9745-0242-ac1100020000"
}

// any pushes to that channel timeout afterwards
//[SOCKET] SwiftPhoenixClient: channel, timeout profile:0000017c-14c7-9745-0242-ac1100020000 21 after 10.0s

What's expected to happen

phx_close needs to take into account join_ref of the channel before closing it

Why it happens

This happens because Phoenix.Socket.V1.JSONSerializer is used right now in SwiftPhoenixClient which doesn't encode join_ref leading to isMember to only do equality checks on channel topics.

How can it be fixed

I can implement a custom serialiser on backend that's a copy of V1 but includes join_ref fields, SwiftPhoenixClient seems to already support them, but I'd rather use V2 serialiser instead on backend that is both slightly more efficient and includes join_ref. For that to happen SwiftPhoenixClient needs to add vsn=2.0.0 to the query string in the websocket url and it also needs to be able to decode the new format, still json, but an array instead of a map. For backwards-compatibility it can be opt-in.

ruslandoga commented 2 years ago

@dsrees if my understanding of the issue is correct, and supporting V2 serialiser would indeed fix it, would you be open to a PR?

dsrees commented 2 years ago

@ruslandoga that would be great. I've never gotten around to supporting the v2 serializer and would appreciate it

dsrees commented 2 years ago

Released in 3.x