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

[Bug]: RunAsync fails to disconnect properly from the gateway #248

Open
Kaoticz opened this issue Oct 30, 2022 · 4 comments
Open

[Bug]: RunAsync fails to disconnect properly from the gateway #248

Kaoticz opened this issue Oct 30, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@Kaoticz
Copy link

Kaoticz commented Oct 30, 2022

Description

DiscordGatewayClient.RunAsync() doesn't seem to be properly disconnecting from the gateway when its CancellationToken is cancelled.

According to the official documentation:

When you close the connection to the gateway with close code 1000 or 1001, your session will be invalidated and your bot will appear offline.

If you simply close the TCP connection or use a different close code, the session will remain active and timeout after a few minutes.

The latter seems to be always happening, no matter what.

Steps to Reproduce

Execute DiscordGatewayClient.RunAsync(), then cancel the CancellationToken passed to it.
RunAsync will stop running, but the bot will still show as online on Discord for a couple of minutes.

Expected Behavior

The bot shuts down and shows as offline on Discord.

Current Behavior

The bot shuts down, but still shows as online on Discord for a couple of minutes.

Library / Runtime Information

.NET: 6.0.110
Remora.Discord: 2022.49.0

@Kaoticz Kaoticz added the bug Something isn't working label Oct 30, 2022
@VelvetToroyashi
Copy link
Contributor

I suspect this line may be the cause, as passing a cancelled token to this method would cause it to throw (which is caught), but that doesn't ensure the socket is properly closed. The correct close code would be sent otherwise.

@VelvetToroyashi
Copy link
Contributor

In fact, very deep down, the CT will actually abort the socket (source), which also disposes of said socket, rendering it unusable. (source)

Interestingly, this is somewhat noted in D#+, which passes CancellationToken.None instead.

This also poses an issue, since iirc the same CancellationToken is passed to ReceiveAsync, which will also abort the socket.

We could do what D#+ does and read in chunks, and check for cancellation before/after reading

@carlst99
Copy link
Contributor

I worked around this in my common gateway refactor by using an internal CancellationTokenSource for receive operations in the WebSocketPayloadTransportService. Said token was cancelled when DisconnectAsync was called, and the receive loop watched for cancellation of the token passed to ReceiveAsync in order to exit as soon as possible. Much like using CancellationToken.None this makes the assumption that websocket messages will be received quickly enough to exit the receive method in a reasonable amount of time.

@VelvetToroyashi
Copy link
Contributor

No promises, but I have little better to do so I'll see about fixing this 👍🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants