Fix duplicate MONITOR listener invocation after async packet processing#3630
Open
Ingrim4 wants to merge 1 commit into
Open
Fix duplicate MONITOR listener invocation after async packet processing#3630Ingrim4 wants to merge 1 commit into
Ingrim4 wants to merge 1 commit into
Conversation
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes two related issues in packet handling after async processing:
MONITORlisteners could be invoked twice when a packet was sent or received after async processing.MONITORpriority.Problem
When async processing completes, ProtocolLib currently uses
sendServerPacket(..., filters = false)orreceiveClientPacket(..., filters = false)to continue packet handling.Using
filters = falseskips normal packet listeners, butMONITORlisteners may still be invoked. Since theMONITORlistener has already run as the final synchronous listener before async processing, this causes it to run twice for the same packet.There is also a packet ordering issue. If at least one synchronous listener is registered for a packet, the packet is delayed by one tick, even if the only synchronous listener is a
MONITORlistener. SinceMONITORlisteners are intended to be read-only and should not modify packets, delaying the packet in that case is unnecessary.In some cases, this delay can break packet ordering. For example, a respawn packet may be sent after later packets, which can cause clients to get stuck waiting indefinitely.
Changes
This PR applies two fixes:
After async packet processing completes, packets are sent or received directly through the injector instead of going through the public
sendServerPacket/receiveClientPacketAPI again.MONITORlistener execution.sendServerPacket/receiveClientPacketno longer delay packets solely because ofMONITORlisteners.MONITORlisteners are still executed on the main thread.Also fixed some deprecations in the injector.