8000 XEP-0215: management of the field `expires` by mid1221213 · Pull Request #1510 · dino/dino · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

XEP-0215: management of the field expires #1510

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 0 commits into from
Closed

XEP-0215: management of the field expires #1510

wants to merge 0 commits into from

Conversation

mid1221213
Copy link
Contributor

Add a field in Xmpp.Xep.ExternalServiceDiscovery to keep track of the expires TURN service value and use it (divided by 2) to restart periodically the external services discovery.

Without this patch, calls (actually ICE) fail to initiate after 24h of Dino uptime because of the expiry of the TURN credentials.

This happened on my setup using prosody (using the default credentials' TTL of 86400) and coturn.

@mid1221213 mid1221213 mentioned this pull request Jan 10, 2024
Closed
@fiaxh
Copy link
Member
fiaxh commented Jan 13, 2024

Thanks for looking into this.

There are some issues:

  • A new Timeout is added whenever a stream is negotiated and they are not cancelled when the stream is broken. That means, there can exist a number of superflurous Timeouts. This is also problematic because the closure captures the stream, preventing it from getting freed.
  • Within the closure, you check whether the account has is connected, but it could be connected via another stream. If the account is connected with a different stream, you would still call the on_stream_negotiated function, but with a broken stream.
  • Please use a DateTime for storing the expired value instead of an int64? - we try to avoid using question-primitives, as accessing them wrongly leads to crashes.
  • Also with those changes, we should rename on_stream_negotiated, since it's not only called on stream negotiation anymore 🙂

@mid1221213
Copy link
Contributor Author

Thanks @fiaxh for your comments. Here is a new version, hopefully better 😉

@fiaxh fiaxh closed this Sep 22, 2024
@fiaxh
Copy link
Member
fiaxh commented Sep 22, 2024

I merged this in 63439cd. Also, I simplified the timeout-removal logic a bit in 3a4ac7c.

(Also, I actually intended to click on merge here, but accidentally closed the PR by pushing a wrong state to it)

@mid1221213 mid1221213 deleted the xep-0215-expires branch September 22, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mid1221213 @fiaxh
0