WICKET-7204 Don't use INJECTION for ByteBuddy proxy creation

[bitstorm]

The current proxy creation with byte buddy uses ClassLoadingStrategy.Default.INJECTION class loading strategy, but this relies on Java Unsafe support, which is deprecated and disabled with the newer release of ByteBuddy.

I'm not a bytecode expert but looks like it's the minimal change required.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.85%. Comparing base (ca5252b) to head (8b8b125).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1577   +/-   ##
=========================================
  Coverage     61.85%   61.85%           
- Complexity    11181    11183    +2     
=========================================
  Files          1245     1245           
  Lines         48220    48227    +7     
  Branches       6759     6759           
=========================================
+ Hits          29825    29830    +5     
- Misses        15694    15696    +2     
  Partials       2701     2701           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[reiern70]

Also not an expert on this. Thus a mild approval

[papegaaij]

I had a close look at this and reproduced two regressions against master, both in wicket-ioc. One of them is silent. The direction is right — moving off INJECTION is worth doing — but I think the strategy is being selected on the wrong property.

1. Package-private methods of a public class are silently no longer intercepted

WRAPPER defines the proxy in a fresh child ClassLoader. WicketProxy_Foo then shares the package name with Foo but sits in a different runtime package, so a package-private method is no longer an override: the call resolves to the superclass method and runs against the Objenesis-allocated proxy, whose fields are all null. No exception, just a wrong value.

That is exactly what WicketNamingStrategy promises in its javadoc — "This way the generated proxy class could still access package-private members of sibling classes" — and what WICKET-7025 added the isPackagePrivate() matcher for.

With a public class exposing a package-private getter:

	proxy.internalGetMessage()
master	"public-concrete"
this branch	null

Test at the bottom (PublicPackagePrivateMethodTest).

2. WICKET-7005 comes back for package-private types

ClassLoadingStrategy.UsingLookup has no allowExistingTypes() — only Default implements Configurable — and TypeCache.findOrInsert is called without a monitor. Concurrent first use of one type therefore defines the class twice:

LinkageError: loader 'app' attempted duplicate class definition for
org.apache.wicket.proxy.packageprivate.WicketProxy_PackagePrivateConcreteObject

A 16-thread stress test on createOrGetProxyClass: 15 of 16 threads fail on this branch, green on master. ParallelInjectionTest does not catch it, because its beans are public static and take the WRAPPER path.

Test at the bottom (PackagePrivateParallelTest).

The condition wants to be about the package, not the visibility

What decides the strategy is not whether the type is public, it is whether the proxy keeps the type's package. WicketNamingStrategy relocates only java.* types, into bytebuddy_generated_wicket_proxy.…. Measured, each in its own JVM:

type	INJECTION (today)	WRAPPER	UsingLookup
public application class	pkg-private intercepted	not intercepted	intercepted
package-private application class	works	inaccessible	works
java.util.ArrayList	works	works	IllegalAccessException — java.base does not open java.util

So java.* → WRAPPER, everything else → UsingLookup.

The inline suggestions carry that, plus a monitor on findOrInsert for #2 and the strategy resolution moved inside the cache lambda (as written it runs privateLookupIn on every createProxy, and throws even when the proxy class is already cached). They are one coherent change — applying only some of them will not compile.

With all of them applied: wicket-ioc 16/16, wicket-spring 54/54, wicket-guice 7/7 green, including ParallelInjectionTest and both new tests. wicket-spring proxies ArrayList<String>, so the java.* branch really is exercised. I did not run a full mvn clean verify.

One caveat I should be honest about: the monitor closes the concurrency race but not soft-reference expunction. TypeCache.Sort.SOFT can drop a cached class while it is still defined in the loader, and regeneration would hit the same LinkageError that allowExistingTypes() used to absorb. Matching the old robustness fully needs a Class.forName recovery or a strong cache.

Smaller points

• UsingLookup ignores the ClassLoader passed to load() — I passed a URLClassLoader and the class still landed in the app loader. So the proxy is defined next to the type rather than into the IClassResolver loader, while DYNAMIC_CLASS_CACHE stays keyed by the resolver's loader. That divergence is inherent to the approach, and it is what makes package access work, but it is worth a thought about classloader lifetime on redeploy.
• On the module path, privateLookupIn needs the target package opened to wicket-ioc's module, where INJECTION did not. Worth a line in the migration guide.
• Both commits have empty bodies, and WICKET-7204 logic refactoring looks like a fixup to squash.
• There is trailing whitespace on six of the new lines (two tabs after createOrGetProxyClass's opening brace, two tab-only blank lines, trailing spaces after the method signature, try and }), and the ternary is indented tabs-then-spaces. The suggestions drop it.

On the premise

I could not reproduce INJECTION being disabled. On JDK 25 with byte-buddy 1.18.13 it still works. It does trigger the sun.misc.Unsafe::objectFieldOffset terminal-deprecation warning — and still does with --add-opens java.base/java.lang=ALL-UNNAMED, so that warning is the Unsafe dispatcher being initialised rather than proof that the definition goes through it. Worth doing regardless, since Unsafe is going away, but it does not look like it is fixing a present breakage at 1.18.13.

Tests

New files, so they cannot be inline suggestions.

wicket-ioc/src/test/java/org/apache/wicket/proxy/util/PublicObjectWithPackagePrivateMethod.java

package org.apache.wicket.proxy.util;

/**
 * A public mock dependency with a package private method, to verify that such methods are
 * intercepted on the proxy as well (WICKET-7025).
 */
public class PublicObjectWithPackagePrivateMethod
{
	private String message;

	public PublicObjectWithPackagePrivateMethod()
	{
	}

	public PublicObjectWithPackagePrivateMethod(final String message)
	{
		this.message = message;
	}

	public String getMessage()
	{
		return message;
	}

	String internalGetMessage()
	{
		return message;
	}
}

wicket-ioc/src/test/java/org/apache/wicket/proxy/util/PublicPackagePrivateMethodTest.java

package org.apache.wicket.proxy.util;

import static org.junit.jupiter.api.Assertions.assertEquals;

import org.apache.wicket.proxy.IProxyTargetLocator;
import org.apache.wicket.proxy.LazyInitProxyFactory;
import org.junit.jupiter.api.Test;

class PublicPackagePrivateMethodTest
{
	private static final PublicObjectWithPackagePrivateMethod TARGET =
		new PublicObjectWithPackagePrivateMethod("public-concrete");

	private static final IProxyTargetLocator LOCATOR = new IProxyTargetLocator()
	{
		private static final long serialVersionUID = 1L;

		@Override
		public Object locateProxyTarget()
		{
			return TARGET;
		}
	};

	@Test
	void packagePrivateMethodOfPublicClassIsIntercepted()
	{
		PublicObjectWithPackagePrivateMethod proxy = (PublicObjectWithPackagePrivateMethod)LazyInitProxyFactory
			.createProxy(PublicObjectWithPackagePrivateMethod.class, LOCATOR);

		assertEquals("public-concrete", proxy.getMessage());
		assertEquals("public-concrete", proxy.internalGetMessage());
	}
}

wicket-ioc/src/test/java/org/apache/wicket/proxy/packageprivate/PackagePrivateParallelTest.java

package org.apache.wicket.proxy.packageprivate;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CountDownLatch;

import org.apache.wicket.proxy.bytebuddy.ByteBuddyProxyFactory;
import org.junit.jupiter.api.Test;

class PackagePrivateParallelTest
{
	@Test
	void concurrentProxyClassCreation() throws Exception
	{
		int n = 16;
		CountDownLatch start = new CountDownLatch(1);
		List<Throwable> failures = new ArrayList<>();
		List<Thread> threads = new ArrayList<>();
		for (int i = 0; i < n; i++)
		{
			Thread t = new Thread(() -> {
				try
				{
					start.await();
					ByteBuddyProxyFactory.createOrGetProxyClass(PackagePrivateConcreteObject.class);
				}
				catch (Throwable e)
				{
					synchronized (failures)
					{
						failures.add(e);
					}
				}
			});
			t.start();
			threads.add(t);
		}
		start.countDown();
		for (Thread t : threads)
		{
			t.join();
		}
		if (!failures.isEmpty())
		{
			throw new AssertionError(failures.size() + "/" + n + " threads failed, first: "
				+ failures.get(0), failures.get(0));
		}
	}
}

[papegaaij]

This bump is separable from the fix: 1.18.13 does not disable INJECTION (I checked on JDK 25), so it is not what forces the change. No objection to bumping, but it reads as part of the fix here.

[bitstorm]

It fails on JDK > 25 (i.e. 26) starting from version 1.18.9. See PR #1574 . So yes, we better keep version bumping out of this commit but I had to do it to test the result on JDK 26.