test: patch Netty's lost-read-interest race in the test server via a javaagent

.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
 

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 }

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}`)

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;
+  }
+}