From 4d419735397574349342f2df025de446149230c8 Mon Sep 17 00:00:00 2001 From: u9g Date: Thu, 27 Aug 2026 03:15:10 -0400 Subject: [PATCH] test: patch Netty's lost-read-interest race in the test server via a javaagent Vanilla servers up to 1.20.1 can stop reading a client's socket during login: Connection.sendPacket calls setAutoRead(false) on the main thread for every PLAY packet sent while the channel still says LOGIN and queues setAutoRead(true) onto the IO thread. Netty re-arms OP_READ only on a 0->1 flip of the flag but the queued clearReadPending0 drops it unconditionally, so when doSendPacket_i runs between the main thread's toggles for i+1 and i+2 the channel ends with autoRead=true and no read interest. Nothing re-arms it; ReadTimeoutHandler kills the connection 30s later and the whole version fails in CI. Mojang removed the toggle in 1.20.2 (MC-265209); Paper's archived branches carry no fix. The agent rewrites AbstractNioChannel.clearReadPending0 with the JDK ClassFile API to leave OP_READ alone when autoRead is already true. It is bytecode-level, so it applies to whatever Netty 4.1.x a server bundles (4.1.9 through 4.1.82 across 1.12-1.20.1) and is a no-op on 1.8.8's Netty 4.0.23, which predates the method. The jar is compiled from test/netty-agent at test time with the JDK's javac and jar and attached through JAVA_TOOL_OPTIONS, so minecraft-wrap is untouched; CI therefore needs a JDK rather than a JRE. Reproduced the stall at ~1% of logins under CPU load and ~10% with Connection DEBUG logging; with the agent, 0 stalls in 800 such logins. --- .github/workflows/ci.yml | 2 +- test/common/nettyAgent.js | 34 +++++++++++ test/externalTest.js | 3 + test/netty-agent/NettyAutoReadFixAgent.java | 67 +++++++++++++++++++++ 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 test/common/nettyAgent.js create mode 100644 test/netty-agent/NettyAutoReadFixAgent.java diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 67c5efcd62..6aa20412e7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,7 +63,7 @@ jobs: uses: actions/setup-java@v1.4.3 with: java-version: 25 - java-package: jre + java-package: jdk - name: Install Dependencies run: npm install diff --git a/test/common/nettyAgent.js b/test/common/nettyAgent.js new file mode 100644 index 0000000000..80aed2eaf5 --- /dev/null +++ b/test/common/nettyAgent.js @@ -0,0 +1,34 @@ +// Builds test/netty-agent into a -javaagent jar. Needs a JDK (javac + jar); +// returns null when there is none so the suite still runs unpatched. +const { execFileSync } = require('child_process') +const fs = require('fs') +const os = require('os') +const path = require('path') + +const SRC = path.join(__dirname, '..', 'netty-agent') +let built + +function tool (name) { + const home = process.env.JAVA_HOME + const candidate = home && path.join(home, 'bin', name) + return candidate && fs.existsSync(candidate) ? candidate : name +} + +function build () { + if (built !== undefined) return built + const out = fs.mkdtempSync(path.join(os.tmpdir(), 'mineflayer-netty-agent-')) + const jar = path.join(out, 'netty-autoread-fix.jar') + const sources = fs.readdirSync(SRC).filter(f => f.endsWith('.java')).map(f => path.join(SRC, f)) + try { + execFileSync(tool('javac'), ['-d', out, ...sources], { stdio: 'pipe' }) + fs.writeFileSync(path.join(out, 'MANIFEST.MF'), 'Premain-Class: NettyAutoReadFixAgent\n') + execFileSync(tool('jar'), ['cfm', jar, path.join(out, 'MANIFEST.MF'), '-C', out, '.'], { stdio: 'pipe' }) + built = jar + } catch (err) { + console.log(`netty agent not built, server runs unpatched: ${err.stderr?.toString().trim() || err.message}`) + built = null + } + return built +} + +module.exports = { build } diff --git a/test/externalTest.js b/test/externalTest.js index 9a7ae45f7c..90845f5787 100644 --- a/test/externalTest.js +++ b/test/externalTest.js @@ -9,6 +9,7 @@ const path = require('path') const { getPort } = require('./common/util') const trace = require('./common/trace') +const nettyAgent = require('./common/nettyAgent') const { once } = require('../lib/promise_utils') // set this to false if you want to test without starting a server automatically @@ -120,6 +121,8 @@ for (const supportedVersion of mineflayer.testedVersions) { } trace.log('server jar downloaded, starting server') propOverrides['server-port'] = PORT + const agent = nettyAgent.build() + if (agent) process.env.JAVA_TOOL_OPTIONS = `${process.env.JAVA_TOOL_OPTIONS ?? ''} -javaagent:${agent}`.trim() wrap.startServer(propOverrides, (err) => { if (err) return done(err) console.log(`pinging ${version.minecraftVersion} port : ${PORT}`) diff --git a/test/netty-agent/NettyAutoReadFixAgent.java b/test/netty-agent/NettyAutoReadFixAgent.java new file mode 100644 index 0000000000..72bd6da6d3 --- /dev/null +++ b/test/netty-agent/NettyAutoReadFixAgent.java @@ -0,0 +1,67 @@ +import java.lang.classfile.ClassFile; +import java.lang.classfile.ClassModel; +import java.lang.classfile.Label; +import java.lang.classfile.MethodModel; +import java.lang.constant.ClassDesc; +import java.lang.constant.ConstantDescs; +import java.lang.constant.MethodTypeDesc; +import java.lang.instrument.ClassFileTransformer; +import java.lang.instrument.Instrumentation; +import java.security.ProtectionDomain; + +/** + * Netty 4.1.x: AbstractNioChannel.clearReadPending0() is queued onto the event + * loop by setAutoRead(false) and drops OP_READ unconditionally when it runs. + * If autoRead has been switched back on in between, the channel is left with + * autoRead=true and no read interest, and nothing ever re-arms it. Rewrites the + * method to leave OP_READ alone when autoRead is already true: + * + * readPending = false; + * if (!config().isAutoRead()) ((AbstractNioUnsafe) unsafe()).removeReadOp(); + */ +public class NettyAutoReadFixAgent { + static final String TARGET = "io/netty/channel/nio/AbstractNioChannel"; + static final ClassDesc CHANNEL = ClassDesc.ofInternalName(TARGET); + static final ClassDesc NIO_UNSAFE = ClassDesc.ofInternalName(TARGET + "$AbstractNioUnsafe"); + static final ClassDesc UNSAFE = ClassDesc.ofInternalName("io/netty/channel/Channel$Unsafe"); + static final ClassDesc CONFIG = ClassDesc.ofInternalName("io/netty/channel/ChannelConfig"); + + public static void premain(String args, Instrumentation inst) { + inst.addTransformer(new ClassFileTransformer() { + public byte[] transform(ClassLoader loader, String name, Class cls, ProtectionDomain pd, byte[] bytes) { + if (!TARGET.equals(name)) return null; + try { + return patch(bytes); + } catch (Throwable t) { + System.err.println("netty-autoread-fix: leaving " + name + " unpatched: " + t); + return null; + } + } + }); + } + + static byte[] patch(byte[] bytes) { + ClassFile cf = ClassFile.of(); + ClassModel cm = cf.parse(bytes); + boolean[] patched = { false }; + byte[] out = cf.transformClass(cm, (cb, ce) -> { + if (ce instanceof MethodModel mm && mm.methodName().equalsString("clearReadPending0")) { + patched[0] = true; + cb.withMethod(mm.methodName(), mm.methodType(), mm.flags().flagsMask(), mb -> mb.withCode(code -> { + Label done = code.newLabel(); + code.aload(0).iconst_0().putfield(CHANNEL, "readPending", ConstantDescs.CD_boolean) + .aload(0).invokevirtual(CHANNEL, "config", MethodTypeDesc.of(CONFIG)) + .invokeinterface(CONFIG, "isAutoRead", MethodTypeDesc.of(ConstantDescs.CD_boolean)) + .ifne(done) + .aload(0).invokevirtual(CHANNEL, "unsafe", MethodTypeDesc.of(UNSAFE)) + .checkcast(NIO_UNSAFE).invokevirtual(NIO_UNSAFE, "removeReadOp", MethodTypeDesc.of(ConstantDescs.CD_void)) + .labelBinding(done).return_(); + })); + } else { + cb.with(ce); + } + }); + System.err.println("netty-autoread-fix: " + (patched[0] ? "patched" : "no clearReadPending0 in") + " " + TARGET); + return patched[0] ? out : null; + } +}