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 incorrect entity scheduler retring issue #181

Closed
wants to merge 1 commit into from

Conversation

MrHua269
Copy link

@MrHua269 MrHua269 commented Jan 7, 2024

No description provided.

@Spottedleaf
Copy link
Member

provide a reason why this change is to be made? why are we avoiding retiring the scheduler of players who have been in the world but are in the change state?

@MrHua269
Copy link
Author

MrHua269 commented Jan 7, 2024

provide a reason why this change is to be made? why are we avoiding retiring the scheduler of players who have been in the world but are in the change state?

It will cause a crash if the player in configuration state and doesn't reply the keepalive from the server for more than 30s.

Like this(offline mode):
C -> S handshake
C -> S loginstart
S -> C loginsuccess
C -> S login acknowledged

S -> C Keepalive
C: Do nothing and keep this state and connection active(send something except keepalive reply) for 30s
and the server will crash when kicked the player for timedout.

I checked the source code and I found this:
image
image
and the same method "onDisconnect" will be called again after kicking in the next tick because the connection is still in the connection list in the global region and when the global region call the method "handleDisconnection",it will call the method "onDisconnect" again and retry the scheduler,then the scheduler throw a exception and the server crashed.Like this:

image

I also tried it on folia 1.20.4 and it still happened

Spottedleaf added a commit that referenced this pull request Jan 9, 2024
The non-game type implementation will not guard against double calls,
which means that any invocation of disconnect() would immediately
call onDisconnect and then later the connection handler would
also call onDisconnect.

Fixes #181
@Spottedleaf
Copy link
Member

Please re-test with e89a107

Due to the issue I described earlier, the actual retire() call needs to be in the common listener to ensure that the scheduler is retired in all cases

@MrHua269
Copy link
Author

It's alright now,Thanks!

@MrHua269 MrHua269 closed this Jan 10, 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.

None yet

2 participants