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 InitialHandler exception handling behaivour #3279

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Outfluencer
Copy link
Collaborator

checking if its possible to send a kick message and sending it to the client is useless here as it would call the delayed close method that would send the packet 250ms delayed but the handlerboss would close it immediately. Also no need to call the channelwrapper close method here because the handler boss will close as i said and we only need to mark closed

@Outfluencer
Copy link
Collaborator Author

@md-5

@Janmm14
Copy link
Contributor

Janmm14 commented Jul 19, 2022

I think we should make it so it still sends disconnect packet.

@Outfluencer
Copy link
Collaborator Author

Outfluencer commented Jul 19, 2022

but it would never do, we need to wait the 250ms to send a kick message otherwise it would be buggy and i think waiting when encountering an exception is not very good.
also i saw that the channelInactive method of the HandlerBoss also markes the channel as closed so we could remove the full method without a difference just less code executed

Edit: my bad, sending it directly with unsafe.sendPacket works in this case, so i will add it and send it without delay, the handler boss will directly close it after we send the kick packet, cause the handler boss will close it instantly anyway waiting 250ms would make no sense at all

otherwise the packet is not sent to the client and remove the other close because the HandlerBoss will close it for as and the inactiv will mark the channel as closed also
@Outfluencer Outfluencer changed the title removed redundant exception handling code fix InitialHandler exception handling behaivour Jul 19, 2022
@Outfluencer
Copy link
Collaborator Author

@md-5
the handler boss will close the connection
https://github.com/SpigotMC/BungeeCord/blob/8a88ce464e0b107b15523109afd7810096e635ca/proxy/src/main/java/net/md_5/bungee/netty/HandlerBoss.java#L210C13-L210C25

after the close, the channelInactive is called, and the channelwrapper is marked as closed.
https://github.com/SpigotMC/BungeeCord/blob/8a88ce464e0b107b15523109afd7810096e635ca/proxy/src/main/java/net/md_5/bungee/netty/HandlerBoss.java#L61C21-L61C31

so closing the channel with the disconnect methode is unnecessary.
it delays the packet sending by 250 ms but the channel is already closed and marked as closed
The kick packet will never be sent to the client.

this pr removes the disconnect method calls and tries to send the kick packet instantly so the client has a chance to see the kick message

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