Skip to content

Fix loading package-info classes#56

Merged
marchermans merged 3 commits intoMcModLauncher:mainfrom
Su5eD:fix/modular-package-info
Jan 12, 2024
Merged

Fix loading package-info classes#56
marchermans merged 3 commits intoMcModLauncher:mainfrom
Su5eD:fix/modular-package-info

Conversation

@Su5eD
Copy link
Copy Markdown
Member

@Su5eD Su5eD commented Nov 25, 2023

The issue

Currently, SJH's ModuleClassLoader is unable to load package-info classes, which are placed in the unnamed module when it calls definePackage.

Java offers 2 ways of defining packages - one with version information and another with a module parameter, which are mutually exclusive. However, our use case requires both package versioning information and the module to be present.

ModuleClassLoader is designed to only load classes from named modules. When calling Class.getPackage().getPackageInfo(), Java will try to load the package-info class using ClassLoader#findClass(String moduleName, String name), passing in null for the moduleName parameter as the package is placed in an unnamed module. Here, MCL expects moduleName to be non-null at all times. However, that is not true according to java's documentation:

Parameters:
moduleName - The module name; or null to find a resource in the unnamed module for this class loader

MCL will happily pass this parameter to Configuration#findModule, which expects a non-null value, resulting in a NPE.

Steps to reproduce

  1. Invoke getPackage().getPackageInfo() on your favourite class, which is loaded by a ModuleClassLoader
  2. Expect a NPE

Expected behavior

Much like other classes, package-info classes should be placed in named modules, too, rather than being loaded in the unnamed module.

The solution

To make java load package-info classes in their module, we must replace the value of the NamedPackage#module field. Since java puts no restrictions in place that would prevent us from setting a named module on a NamedPackage instance, we reflectively replace the default unnamed module value with the package's actual module.

Step 1: Grab the trusted method lookup instance and create a field handle for the package module field

var trustedLookupField = MethodHandles.Lookup.class.getDeclaredField("IMPL_LOOKUP");  
trustedLookupField.setAccessible(true);  
MethodHandles.Lookup trustedLookup = (MethodHandles.Lookup) trustedLookupField.get(null);  
  
Class<?> namedPackage = Class.forName("java.lang.NamedPackage");  
PKG_MODULE_HANDLE = trustedLookup.findVarHandle(namedPackage, "module", Module.class);

Step 2: After defining each class (necessary for the module to be known), ensure its package is placed in a named module

// Get package module
Module value = (Module) PKG_MODULE_HANDLE.get(pkg);  
// Check if the value is not a named module
if (value == null || !value.isNamed()) {  
    try {
        // Replace the value with the loaded class's module
        PKG_MODULE_HANDLE.set(pkg, module);  
    } catch (Throwable t) {  
        throw new RuntimeException(t);  
    }  
}

@Su5eD Su5eD added the bug Something isn't working label Nov 25, 2023
@Technici4n
Copy link
Copy Markdown
Contributor

I can't say I understand the issue. Can you add a relevant regression test?

@Su5eD Su5eD force-pushed the fix/modular-package-info branch from 4955c69 to 9a052fb Compare November 27, 2023 20:25
@marchermans marchermans merged commit fd3fa25 into McModLauncher:main Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants