Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ClassLoaderFixer to v2 #29

Open
wants to merge 1 commit into
base: 1.3.2-forge
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 104 additions & 62 deletions src/main/java/cpw/mods/fml/relauncher/RelaunchClassLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@
package cpw.mods.fml.relauncher;

import java.net.*;
import java.security.CodeSigner;
import java.security.CodeSource;
import java.util.jar.Attributes.Name;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like an unnecessary style chane, to change this to an inner class import.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary? Might be. Just copy-pasted from my ClassLoaderFixer source code.

import java.util.jar.Attributes;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.jar.Manifest;

import cpw.mods.fml.common.FMLLog;
import fr.catcore.fabricatedforge.util.Utils;
import fr.catcore.modremapperapi.ClassTransformer;
import net.fabricmc.loader.impl.launch.FabricLauncherBase;
Expand All @@ -33,8 +31,7 @@
import java.util.logging.Level;

public class RelaunchClassLoader extends URLClassLoader {
// ClassLoaderFixer indication
public static String FIXER_VERSION = "1";
public static String FIXER_VERSION = "2";
// Left behind for CCC/NEI compatibility
private static String[] excludedPackages = new String[0];
// Left behind for CCC/NEI compatibility
Expand All @@ -46,45 +43,55 @@ public class RelaunchClassLoader extends URLClassLoader {
private Set<String> classLoaderExceptions = new HashSet<>();
private Set<String> transformerExceptions = new HashSet<>();

private static final boolean DEBUG_CLASSLOADING = Boolean.parseBoolean(System.getProperty("fml.debugClassLoading",
"false"));

public RelaunchClassLoader() {
super(new URL[0], FMLRelauncher.class.getClassLoader());
this.sources = new ArrayList<>();
this.parent = this.getClass().getClassLoader();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'mon man.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh, get off your high horse. If you think this is unnecessary, just close this PR without any comment. But don't spend your time writing such comments if the explanation is already there in the premise of the PR.

this.parent = getClass().getClassLoader();
this.cachedClasses = new HashMap<>(1000);
this.transformers = new ArrayList<>(2);
// ReflectionHelper.setPrivateValue(ClassLoader.class, null, this, "scl");
Thread.currentThread().setContextClassLoader(this);

// standard classloader exclusions
this.addClassLoaderExclusion("java.");
this.addClassLoaderExclusion("sun.");
this.addClassLoaderExclusion("net.minecraftforge.classloading.");
addClassLoaderExclusion("java.");
addClassLoaderExclusion("sun.");
addClassLoaderExclusion("org.lwjgl.");
addClassLoaderExclusion("cpw.mods.fml.relauncher.");
addClassLoaderExclusion("net.minecraftforge.classloading.");

// standard transformer exclusions
this.addTransformerExclusion("javax.");
this.addTransformerExclusion("org.objectweb.asm.");
this.addTransformerExclusion("com.google.common.");
this.addTransformerExclusion("cpw.mods.fml.common.asm.SideOnly");
this.addTransformerExclusion("cpw.mods.fml.common.Side");
this.addTransformerExclusion("fr.catcore.fabricatedforge.");
this.addClassLoaderExclusion("com.llamalad7.mixinextras.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did these become static? If not, just leave it be...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

addTransformerExclusion("javax.");
addTransformerExclusion("org.objectweb.asm.");
addTransformerExclusion("com.google.common.");
addTransformerExclusion("cpw.mods.fml.common.asm.SideOnly");
addTransformerExclusion("cpw.mods.fml.common.Side");
addTransformerExclusion("fr.catcore.fabricatedforge.");
addClassLoaderExclusion("com.llamalad7.mixinextras.");
}

public void registerTransformer(String transformerClassName) {
try {
IClassTransformer classTransformer = (IClassTransformer) Class.forName(transformerClassName).newInstance();
ClassTransformer.registerPostTransformer(classTransformer);
System.out.println("Registered ClassTransformer: " + transformerClassName);
this.transformers.add(classTransformer);
} catch (Exception var3) {
FMLRelaunchLog.log(Level.SEVERE, var3, "A critical problem occurred registering the ASM transformer class %s", transformerClassName);
transformers.add(classTransformer);
} catch (Exception e) {
FMLRelaunchLog.log(Level.SEVERE, e, "A critical problem occurred registering the ASM transformer class %s",
transformerClassName);
}

}

@Override
public Class<?> findClass(String name) throws ClassNotFoundException {
return Class.forName(name, false, this.parent);
// if (invalidClasses.contains(name))
// {
// throw new ClassNotFoundException(name);
// }
// // NEI/CCC compatibility code
// if (excludedPackages.length != 0)
// {
Expand Down Expand Up @@ -114,9 +121,17 @@ public Class<?> findClass(String name) throws ClassNotFoundException {
// {
// if (name.startsWith(st))
// {
// Class<?> cl = super.findClass(name);
// cachedClasses.put(name, cl);
// return cl;
// try
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point in adding yet more commented out code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep the codebase consistent. Explains the working principle behind ClassLoaderFixer. If it's unnecessary, then this entire commented out code snippet should have already been removed ages ago

// {
// Class<?> cl = super.findClass(name);
// cachedClasses.put(name, cl);
// return cl;
// }
// catch (ClassNotFoundException e)
// {
// invalidClasses.add(name);
// throw e;
// }
// }
// }
//
Expand All @@ -126,6 +141,7 @@ public Class<?> findClass(String name) throws ClassNotFoundException {
// int lastDot = name.lastIndexOf('.');
// String pkgname = lastDot == -1 ? "" : name.substring(0, lastDot);
// String fName = name.replace('.', '/').concat(".class");
// String pkgPath = pkgname.replace('.', '/');
// URLConnection urlConnection = findCodeSourceConnectionFor(fName);
// if (urlConnection instanceof JarURLConnection && lastDot > -1)
// {
Expand All @@ -138,35 +154,54 @@ public Class<?> findClass(String name) throws ClassNotFoundException {
// Package pkg = getPackage(pkgname);
// getClassBytes(name);
// signers = ent.getCodeSigners();
// if (pkg != null)
// if (pkg == null)
// {
// pkg = definePackage(pkgname, mf, jarUrlConn.getJarFileURL());
// packageManifests.put(pkg, mf);
// }
// else
// {
// if (pkg.isSealed() && !pkg.isSealed(jarUrlConn.getJarFileURL()))
// {
// FMLRelaunchLog.severe("The jar file %s is trying to seal already secured path %s", jf.getName(), pkgname);
// FMLLog.severe("The jar file %s is trying to seal already secured path %s", jf.getName()
// , pkgname);
// }
// else if (isSealed(pkgname, mf))
// {
// FMLRelaunchLog.severe("The jar file %s has a security seal for path %s, but that path is defined and not secure", jf.getName(), pkgname);
// FMLLog.severe("The jar file %s has a security seal for path %s, but that path is
// defined and not secure", jf.getName(), pkgname);
// }
// }
// }
// }
// else if (lastDot > -1)
// {
// if (getPackage(pkgname)==null)
// Package pkg = getPackage(pkgname);
// if (pkg == null)
// {
// pkg = definePackage(pkgname, null, null, null, null, null, null, null);
// packageManifests.put(pkg, EMPTY);
// }
// else if (pkg.isSealed())
// {
// definePackage(pkgname, null, null, null, null, null, null, null);
// FMLLog.severe("The URL %s is defining elements for sealed path %s", urlConnection.getURL(),
// pkgname);
// }
// }
// byte[] basicClass = getClassBytes(name);
// byte[] transformedClass = runTransformers(name, basicClass);
// URL url = urlConnection == null ? null : urlConnection.getURL();
// Class<?> cl = defineClass(name, transformedClass, 0, transformedClass.length, new CodeSource(url, signers));
// Class<?> cl = defineClass(name, transformedClass, 0, transformedClass.length, new CodeSource
// (urlConnection.getURL(), signers));
// cachedClasses.put(name, cl);
// return cl;
// }
// catch (Throwable e)
// {
// invalidClasses.add(name);
// if (DEBUG_CLASSLOADING)
// {
// FMLLog.log(Level.FINEST, e, "Exception encountered attempting classloading of %s", name);
// }
// throw new ClassNotFoundException(name, e);
// }
}
Expand All @@ -176,11 +211,11 @@ private boolean isSealed(String path, Manifest man)
Attributes attr = man.getAttributes(path);
String sealed = null;
if (attr != null) {
sealed = attr.getValue(Attributes.Name.SEALED);
sealed = attr.getValue(Name.SEALED);
}
if (sealed == null) {
if ((attr = man.getMainAttributes()) != null) {
sealed = attr.getValue(Attributes.Name.SEALED);
sealed = attr.getValue(Name.SEALED);
}
}
return "true".equalsIgnoreCase(sealed);
Expand All @@ -206,29 +241,6 @@ private URLConnection findCodeSourceConnectionFor(String name)
}
}

public byte[] getClassBytes(String name) throws IOException {
InputStream classStream = null;

try {
URL classResource = ((URLClassLoader)this.parent.getParent()).findResource(name.replace('.', '/').concat(".class"));
if (classResource == null) {
return null;
}

classStream = classResource.openStream();
return readFully(classStream);
} finally {
if (classStream != null) {
try {
classStream.close();
} catch (IOException var12) {
// Swallow the close exception
}
}

}
}

private byte[] runTransformers(String name, byte[] basicClass) {
for (IClassTransformer transformer : transformers)
{
Expand All @@ -237,16 +249,17 @@ private byte[] runTransformers(String name, byte[] basicClass) {
return basicClass;
}

@Override
public void addURL(URL url) {
super.addURL(url);

FabricLauncherBase.getLauncher().addToClassPath(UrlUtil.asPath(url));

this.sources.add(url);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, why would you remove this? It's making the type of call clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as one of the above here

sources.add(url);
}

public List<URL> getSources() {
return this.sources;
return sources;
}

private byte[] readFully(InputStream stream) {
Expand All @@ -259,23 +272,52 @@ private byte[] readFully(InputStream stream) {
}

return bos.toByteArray();
} catch (Throwable var4) {
/// HMMM
} catch (Throwable t) {
FMLLog.log(Level.WARNING, t, "Problem loading class");
return new byte[0];
}
}

public List<IClassTransformer> getTransformers() {
return Collections.unmodifiableList(this.transformers);
return Collections.unmodifiableList(transformers);
}

private void addClassLoaderExclusion(String toExclude) {
this.classLoaderExceptions.add(toExclude);
classLoaderExceptions.add(toExclude);
Utils.TRANSFORMER_EXCLUSIONS.add(toExclude);
}

void addTransformerExclusion(String toExclude) {
this.transformerExceptions.add(toExclude);
transformerExceptions.add(toExclude);
Utils.TRANSFORMER_EXCLUSIONS.add(toExclude);
}

public byte[] getClassBytes(String name) throws IOException {
InputStream classStream = null;

try {
URL classResource = ((URLClassLoader)this.parent.getParent()).findResource(name.replace('.', '/').concat(".class"));
if (classResource == null) {
if (DEBUG_CLASSLOADING) {
FMLLog.finest("Failed to find class resource %s", name.replace('.', '/').concat(".class"));
}
return null;
}

classStream = classResource.openStream();
if (DEBUG_CLASSLOADING) {
FMLLog.finest("Loading class %s from resource %s", name, classResource.toString());
}
return readFully(classStream);
} finally {
if (classStream != null) {
try {
classStream.close();
} catch (IOException e) {
// Swallow the close exception
}
}

}
}
}
Loading