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

Fix unsafe socket usage (v2) #837

Merged

Conversation

saurtron
Copy link
Contributor

@saurtron saurtron commented Dec 1, 2024

Work done

  • New fix for sockets (see below for more information).
  • Fixed version of Fix not closing sockets and bad timeout error condition checks. #827 that was merged and then reverted because of api_analytics issue affecting the join queue.
  • Pbbly good idea to test online by devs before deploying to all users.
    • I tested and queue works well now, also could join a game as spec.

Explanation about regression with previous fix

(note the same fix applied to base Chobby and zk Chobby didn't have this problem since they don't have the api_analytics module)

Turns out analytics is doing more than expected (not the module I'd have expected to produce actual lobby issues).

Due to a logic error, when the initial socket is disposed of (and now set to nil), once lobby is connected it would stop sending information, and apparently that's what makes the queue status show ok.

Seems analytics is sending one way when lobby is not connected, and when it's connected it sends in a different way.

For those curious, this is the fix for the issue: c5d87c1

More info the socket fixes:

  • Fix not closing sockets consistently at api_analytics.lua, api_spring_launcher.lua and interface_shared.lua.
  • Also fixes some error condition checks on client:connect calls.
    • They would result in no error be treated as an error since lua doing not res and not res == "timeout" becomes not res and (not res) == "timeout" (never true) due to operator precedence.
    • Also it was checking for the error at res, when it's actually at err when doing: res, err = client.connect(...)

Related issues

@saurtron saurtron changed the title Fix unsafe socket usage fixed Fix unsafe socket usage (v2) Dec 1, 2024
@AntlerForce AntlerForce merged commit 32efee5 into beyond-all-reason:master Dec 9, 2024
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.

2 participants