diff --git a/common/src/main/java/xyz/jonesdev/sonar/common/netty/TailExceptionsHandler.java b/common/src/main/java/xyz/jonesdev/sonar/common/netty/TailExceptionsHandler.java index 606f2401a..d2a173182 100644 --- a/common/src/main/java/xyz/jonesdev/sonar/common/netty/TailExceptionsHandler.java +++ b/common/src/main/java/xyz/jonesdev/sonar/common/netty/TailExceptionsHandler.java @@ -21,6 +21,7 @@ import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; import org.jetbrains.annotations.NotNull; +import xyz.jonesdev.sonar.common.util.ProtocolUtil; @ChannelHandler.Sharable public final class TailExceptionsHandler extends ChannelDuplexHandler { @@ -33,6 +34,20 @@ public final class TailExceptionsHandler extends ChannelDuplexHandler { // Additionally, this will also run after our custom decoder. @Override public void exceptionCaught(final @NotNull ChannelHandlerContext ctx, final Throwable cause) throws Exception { + // If ProtocolUtil#closeWith(...) already queued a graceful write-then-close for + // this channel (e.g. VerificationHandler#fail() calls disconnect() and then + // throws to unwind), don't race it with a second, immediate close() here. Doing + // so can abort the still-in-flight kick/disconnect packet write before it's fully + // flushed, so the client only receives a truncated packet instead of our message. + // The scheduled close from closeWith() will still run once the write completes. + // See: https://github.com/jonesdevelopment/sonar/issues/476 + if (Boolean.TRUE.equals(ctx.channel().attr(ProtocolUtil.CLOSING).get())) { + if (LOG_EXCEPTIONS) { + cause.printStackTrace(System.err); + } + return; + } + // Close the channel if we encounter any errors. ctx.close(); if (LOG_EXCEPTIONS) { diff --git a/common/src/main/java/xyz/jonesdev/sonar/common/util/ProtocolUtil.java b/common/src/main/java/xyz/jonesdev/sonar/common/util/ProtocolUtil.java index eca3824d8..dcced1fea 100644 --- a/common/src/main/java/xyz/jonesdev/sonar/common/util/ProtocolUtil.java +++ b/common/src/main/java/xyz/jonesdev/sonar/common/util/ProtocolUtil.java @@ -24,6 +24,7 @@ import io.netty.channel.ChannelFutureListener; import io.netty.handler.codec.DecoderException; import io.netty.handler.codec.EncoderException; +import io.netty.util.AttributeKey; import io.netty.util.Version; import lombok.experimental.UtilityClass; import net.kyori.adventure.nbt.BinaryTag; @@ -217,9 +218,20 @@ public static void checkNettyVersion() { } } + public static final AttributeKey CLOSING = AttributeKey.valueOf("sonar-graceful-close-in-progress"); + public static void closeWith(final @NotNull Channel channel, final @NotNull ProtocolVersion protocolVersion, final @NotNull Object msg) { + // Mark this channel as already undergoing a graceful close *before* we queue the + // write. If something downstream (e.g. an exception thrown right after this call, + // such as VerificationHandler#fail()'s QuietDecoderException) reaches + // TailExceptionsHandler while our writeAndFlush(...).addListener(CLOSE) is still + // in flight, that handler must not call ctx.close() a second time - doing so can + // abort the still-pending write and deliver only a truncated packet to the client + // (which then sees a bare disconnect/reset instead of our kick message). + // See: https://github.com/jonesdevelopment/sonar/issues/476 + channel.attr(CLOSING).set(true); if (protocolVersion.lessThan(ProtocolVersion.MINECRAFT_1_8)) { channel.eventLoop().execute(() -> { channel.config().setAutoRead(false);