Use less reflection on Velocity and fixed ChildFirstClassLoader

buildSrc/src/main/kotlin/extensions.kt

@@ -26,7 +26,6 @@
 import net.kyori.indra.git.IndraGitExtension
 import org.gradle.api.Project
 import org.gradle.api.artifacts.MinimalExternalModuleDependency
-import org.gradle.api.artifacts.ProjectDependency
 import org.gradle.api.provider.Provider
 import org.gradle.kotlin.dsl.the
 
@@ -59,30 +58,24 @@ fun buildNumberAsString(): String =
 val providedDependencies = mutableMapOf<String, MutableSet<Pair<String, Any>>>()
 val relocatedPackages = mutableMapOf<String, MutableSet<String>>()
 
-fun Project.provided(pattern: String, name: String, version: String, excludedOn: Int = 0b110) {
- val format = "${calcExclusion(pattern, 0b100, excludedOn)}:" +
- "${calcExclusion(name, 0b10, excludedOn)}:" +
- calcExclusion(version, 0b1, excludedOn)
-
+fun Project.providedDependency(dependency: MinimalExternalModuleDependency) {
+ val format = "${dependency.group}:${dependency.name}:"
  providedDependencies.getOrPut(project.name) { mutableSetOf() }.add(Pair(format, format))
- dependencies.add("compileOnlyApi", "$pattern:$name:$version")
 }
 
-fun Project.provided(dependency: ProjectDependency) {
- providedDependencies.getOrPut(project.name) { mutableSetOf() }
- .add(Pair(dependency.group + ":" + dependency.name, dependency))
- dependencies.add("compileOnlyApi", dependency)
-}
+fun Project.providedDependency(provider: Provider<MinimalExternalModuleDependency>) =
+ providedDependency(provider.get())
 
-fun Project.provided(dependency: MinimalExternalModuleDependency) =
- provided(dependency.module.group, dependency.module.name, dependency.versionConstraint.requiredVersion)
+fun Project.provided(dependency: MinimalExternalModuleDependency) {
+ providedDependency(dependency)
+ dependencies.add("compileOnlyApi",
+ "${dependency.group}:${dependency.name}:${dependency.versionConstraint.requiredVersion}"
+ )
+}
 
 fun Project.provided(provider: Provider<MinimalExternalModuleDependency>) =
  provided(provider.get())
 
 fun Project.relocate(pattern: String) =
  relocatedPackages.getOrPut(project.name) { mutableSetOf() }
  .add(pattern)
-
-private fun calcExclusion(section: String, bit: Int, excludedOn: Int): String =
- if (excludedOn and bit > 0) section else ""

buildSrc/src/main/kotlin/floodgate.shadow-conventions.gradle.kts

@@ -21,8 +21,8 @@ tasks {
  doFirst {
  mergeServiceFiles()
 
- providedDependencies[project.name]?.forEach { (name, notation) ->
- sJar.dependencies {
+ sJar.dependencies {
+ providedDependencies[project.name]?.forEach { (name, notation) ->
  println("Excluding $name from ${project.name}")
  exclude(dependency(notation))
  }

core/build.gradle.kts

@@ -41,7 +41,9 @@ dependencies {
 // present on all platforms
 provided(libs.netty.transport)
 provided(libs.netty.codec)
+providedDependency(libs.slf4j)
 
+// we're isolated but bstats doesn't know that
 relocate("org.bstats")
 
 tasks {

gradle.properties

@@ -2,5 +2,5 @@ org.gradle.configureondemand=true
 org.gradle.caching=true
 org.gradle.parallel=true
 
-version=2.2.2-SNAPSHOT
+version=3.0.0-SNAPSHOT
 micronautVersion=4.3.1

gradle/libs.versions.toml

@@ -28,7 +28,7 @@ paper = "1.20.2-R0.1-SNAPSHOT"
 authlib = "5.0.47"
 
 # velocity
-velocity = "3.2.0-SNAPSHOT"
+velocity = "3.3.0-SNAPSHOT"
 
 # buildSrc
 indra = "3.1.3"
@@ -93,6 +93,7 @@ authlib = { module = "com.mojang:authlib", version.ref = "authlib" }
 # velocity
 cloud-velocity = { module = "org.incendo:cloud-velocity", version.ref = "cloud" }
 velocity-api = { module = "com.velocitypowered:velocity-api", version.ref = "velocity" }
+velocity-proxy = { module = "com.velocitypowered:velocity-proxy", version.ref = "velocity" }
 
 # buildSrc
 checker-qual = { module = "org.checkerframework:checker-qual", version.ref = "checkerframework" }

isolation/src/main/java/org/geysermc/floodgate/isolation/library/classloader/LibraryClassLoader.java

@@ -61,4 +61,8 @@ public Set<Path> loadedPaths() {
  public boolean isLoaded(Path path) {
  return loadedPaths.contains(path);
  }
+
+ static {
+ ClassLoader.registerAsParallelCapable();
+ }
 }

isolation/src/main/java/org/geysermc/floodgate/isolation/util/ChildFirstClassLoader.java

@@ -28,98 +28,65 @@
 import java.io.IOException;
 import java.net.URL;
 import java.util.Enumeration;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
 import java.util.Objects;
 import org.geysermc.floodgate.isolation.library.classloader.LibraryClassLoader;
 
-// Based of a Medium article https://medium.com/@isuru89/java-a-child-first-class-loader-cbd9c3d0305
 public class ChildFirstClassLoader extends LibraryClassLoader {
- private final ClassLoader systemClassLoader;
-
  public ChildFirstClassLoader(ClassLoader parent) {
  super(Objects.requireNonNull(parent));
- systemClassLoader = getSystemClassLoader();
  }
 
  @Override
  protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
- Class<?> loadedClass = findLoadedClass(name);
- if (loadedClass == null) {
- try {
- if (systemClassLoader != null) {
- loadedClass = systemClassLoader.loadClass(name);
- }
- } catch (ClassNotFoundException ignored) {}
-
- try {
- if (loadedClass == null) {
+ synchronized (getClassLoadingLock(name)) {
+ Class<?> loadedClass = findLoadedClass(name);
+ if (loadedClass == null) {
+ // actual Java system classes (like java.lang.Integer) should be loaded by the parent classloader,
+ // which does use the system class loader (unlike us). findClass only returns its own classes.
+ // We don't call the system class loader ourselves since e.g. on Velocity, Velocity is the system.
+ // This resulted in Velocity overriding our own Configurate version for example
+ try {
  loadedClass = findClass(name);
+ } catch (ClassNotFoundException ignored) {
+ loadedClass = super.loadClass(name, resolve);
  }
- } catch (ClassNotFoundException e) {
- loadedClass = super.loadClass(name, resolve);
  }
- }
 
- if (resolve) {
- resolveClass(loadedClass);
+ if (resolve) {
+ resolveClass(loadedClass);
+ }
+ return loadedClass;
  }
- return loadedClass;
  }
 
  @Override
  public Enumeration<URL> getResources(String name) throws IOException {
- List<URL> allResources = new LinkedList<>();
-
- Enumeration<URL> systemResources = systemClassLoader.getResources(name);
- if (systemResources != null) {
- while (systemResources.hasMoreElements()) {
- allResources.add(systemResources.nextElement());
- }
- }
-
- Enumeration<URL> thisResources = findResources(name);
- if (thisResources != null) {
- while (thisResources.hasMoreElements()) {
- allResources.add(thisResources.nextElement());
- }
- }
-
- Enumeration<URL> parentResources = getParent().getResources(name);
- if (parentResources != null) {
- while (parentResources.hasMoreElements()) {
- allResources.add(parentResources.nextElement());
- }
- }
-
  return new Enumeration<>() {
- final Iterator<URL> it = allResources.iterator();
+ final Enumeration<URL> thisResources = findResources(name);
+ final Enumeration<URL> parentResources = getParent().getResources(name);
 
  @Override
  public boolean hasMoreElements() {
- return it.hasNext();
+ return thisResources.hasMoreElements() || parentResources.hasMoreElements();
  }
 
  @Override
  public URL nextElement() {
- return it.next();
+ return thisResources.hasMoreElements() ? thisResources.nextElement() : parentResources.nextElement();
  }
  };
  }
 
  @Override
  public URL getResource(String name) {
- URL resource = null;
- if (systemClassLoader != null) {
- resource = systemClassLoader.getResource(name);
- }
- if (resource == null) {
- resource = findResource(name);
- }
+ URL resource = findResource(name);
  if (resource == null) {
  resource = getParent().getResource(name);
  }
  return resource;
  }
+
+ static {
+ ClassLoader.registerAsParallelCapable();
+ }
 }

velocity/base/build.gradle.kts

@@ -17,3 +17,5 @@ relocate("org.yaml.snakeyaml")
 // these dependencies are already present on the platform
 provided(libs.gson)
 provided(libs.velocity.api)
+provided(libs.velocity.proxy)
+providedDependency(libs.slf4j)

velocity/base/src/main/java/org/geysermc/floodgate/velocity/addon/data/VelocityProxyDataHandler.java

@@ -26,18 +26,18 @@
 package org.geysermc.floodgate.velocity.addon.data;
 
 import static java.util.Objects.requireNonNull;
-import static org.geysermc.floodgate.core.util.ReflectionUtils.getCastedValue;
-import static org.geysermc.floodgate.core.util.ReflectionUtils.getClassOrFallbackPrefixed;
 import static org.geysermc.floodgate.core.util.ReflectionUtils.getField;
-import static org.geysermc.floodgate.core.util.ReflectionUtils.getMethodByName;
 import static org.geysermc.floodgate.core.util.ReflectionUtils.getPrefixedClass;
-import static org.geysermc.floodgate.core.util.ReflectionUtils.invoke;
 import static org.geysermc.floodgate.core.util.ReflectionUtils.setValue;
 
+import com.velocitypowered.proxy.connection.MinecraftConnection;
+import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
+import com.velocitypowered.proxy.connection.client.InitialLoginSessionHandler;
+import com.velocitypowered.proxy.protocol.packet.HandshakePacket;
+import com.velocitypowered.proxy.protocol.packet.ServerLoginPacket;
 import io.netty.channel.Channel;
 import io.netty.util.AttributeKey;
 import java.lang.reflect.Field;
-import java.lang.reflect.Method;
 import java.net.InetSocketAddress;
 import org.geysermc.api.connection.Connection;
 import org.geysermc.floodgate.core.addon.data.CommonNettyDataHandler;
@@ -49,51 +49,16 @@
 import org.geysermc.floodgate.core.logger.FloodgateLogger;
 
 public final class VelocityProxyDataHandler extends CommonNettyDataHandler {
- private static final Field HANDSHAKE;
- private static final Class<?> HANDSHAKE_PACKET;
- private static final Field HANDSHAKE_SERVER_ADDRESS;
  private static final Field REMOTE_ADDRESS;
 
- private static final Class<?> SERVER_LOGIN_PACKET;
- private static final Method GET_SESSION_HANDLER;
  private static final Class<?> INITIAL_LOGIN_SESSION_HANDLER;
  private static final Field FORCE_KEY_AUTHENTICATION;
 
  static {
- Class<?> iic = getPrefixedClass("connection.client.InitialInboundConnection");
- requireNonNull(iic, "InitialInboundConnection class cannot be null");
-
- HANDSHAKE = getField(iic, "handshake");
- requireNonNull(HANDSHAKE, "Handshake field cannot be null");
-
- HANDSHAKE_PACKET = getClassOrFallbackPrefixed(
- "protocol.packet.HandshakePacket",
- "protocol.packet.Handshake"
- );
- requireNonNull(HANDSHAKE_PACKET, "Handshake packet class cannot be null");
-
- HANDSHAKE_SERVER_ADDRESS = getField(HANDSHAKE_PACKET, "serverAddress");
- requireNonNull(HANDSHAKE_SERVER_ADDRESS, "Address in the Handshake packet cannot be null");
-
  Class<?> minecraftConnection = getPrefixedClass("connection.MinecraftConnection");
  REMOTE_ADDRESS = getField(minecraftConnection, "remoteAddress");
  requireNonNull(REMOTE_ADDRESS, "remoteAddress cannot be null");
 
- SERVER_LOGIN_PACKET = getClassOrFallbackPrefixed(
- "protocol.packet.ServerLoginPacket",
- "protocol.packet.ServerLogin"
- );
- requireNonNull(SERVER_LOGIN_PACKET, "ServerLogin packet class cannot be null");
-
-
- Method sessionHandler = getMethodByName(minecraftConnection, "getActiveSessionHandler", true);
- if (sessionHandler == null) {
- // We are pre-1.20.2
- sessionHandler = getMethodByName(minecraftConnection, "getSessionHandler", true);
- }
- GET_SESSION_HANDLER = sessionHandler;
- requireNonNull(GET_SESSION_HANDLER, "getSessionHandler method cannot be null");
-
  INITIAL_LOGIN_SESSION_HANDLER = getPrefixedClass("connection.client.InitialLoginSessionHandler");
  requireNonNull(INITIAL_LOGIN_SESSION_HANDLER, "InitialLoginSessionHandler cannot be null");
 
@@ -122,7 +87,7 @@ protected void setNewIp(Channel channel, InetSocketAddress newIp) {
 
  @Override
  protected Object setHostname(Object handshakePacket, String hostname) {
- setValue(handshakePacket, HANDSHAKE_SERVER_ADDRESS, hostname);
+ ((HandshakePacket) handshakePacket).setServerAddress(hostname);
  return handshakePacket;
  }
 
@@ -138,23 +103,21 @@ protected boolean shouldRemoveHandler(HandleResult result) {
 
  @Override
  public boolean channelRead(Object packet) {
- if (HANDSHAKE_PACKET.isInstance(packet)) {
- handle(packet, getCastedValue(packet, HANDSHAKE_SERVER_ADDRESS));
+ if (packet instanceof HandshakePacket handshake) {
+ handle(packet, handshake.getServerAddress());
  // otherwise, it'll get read twice. once by the packet queue and once by this method
  return false;
  }
 
  // at this point we know that forceKeyAuthentication is enabled
- if (SERVER_LOGIN_PACKET.isInstance(packet)) {
- Object minecraftConnection = ctx.pipeline().get("handler");
- Object sessionHandler = invoke(minecraftConnection, GET_SESSION_HANDLER);
- if (!INITIAL_LOGIN_SESSION_HANDLER.isInstance(sessionHandler)) {
+ if (packet instanceof ServerLoginPacket) {
+ MinecraftConnection minecraftConnection = (MinecraftConnection) ctx.pipeline().get("handler");
+ MinecraftSessionHandler sessionHandler = minecraftConnection.getActiveSessionHandler();
+ if (!(sessionHandler instanceof InitialLoginSessionHandler)) {
  logger.error("Expected player's session handler to be InitialLoginSessionHandler");
  return true;
  }
- if (FORCE_KEY_AUTHENTICATION != null) {
- setValue(sessionHandler, FORCE_KEY_AUTHENTICATION, false);
- }
+ setValue(sessionHandler, FORCE_KEY_AUTHENTICATION, false);
  }
  return true;
  }