byte-buddy icon indicating copy to clipboard operation
byte-buddy copied to clipboard

Parallel capable classloader and concurrent defineClass

Open beikov opened this issue 6 months ago • 6 comments

Some Wildfly tests recently hit a problem that has ByteBuddy in the stack trace and I wanted to share it with you to see if this might ring a bell. Since it is concurrency related, it's hard to put a finger on specific code at fault.

The Wildfly class loader is parallel capable and during application deployment, some threads run into the following error:

Caused by: java.lang.IllegalStateException: There is no field invocationTarget$n2rb551 defined on class org.jboss.as.test.integration.jpa.transaction.Company$HibernateAccessOptimizer5employees
	at [email protected]//net.bytebuddy.implementation.LoadedTypeInitializer$ForStaticField.onLoad(LoadedTypeInitializer.java:174)
	at [email protected]//net.bytebuddy.implementation.LoadedTypeInitializer$Compound.onLoad(LoadedTypeInitializer.java:234)
	at [email protected]//net.bytebuddy.dynamic.TypeResolutionStrategy$Passive.initialize(TypeResolutionStrategy.java:103)
	at [email protected]//net.bytebuddy.dynamic.DynamicType$Default$Unloaded.load(DynamicType.java:6424)
	at [email protected]//org.hibernate.bytecode.internal.bytebuddy.ByteBuddyState.load(ByteBuddyState.java:239)
	at [email protected]//org.hibernate.bytecode.internal.bytebuddy.BytecodeProviderImpl.getReflectionOptimizer(BytecodeProviderImpl.java:260)
	at [email protected]//org.hibernate.metamodel.internal.EntityRepresentationStrategyPojoStandard.resolveReflectionOptimizer(EntityRepresentationStrategyPojoStandard.java:325)
	at [email protected]//org.hibernate.metamodel.internal.EntityRepresentationStrategyPojoStandard.<init>(EntityRepresentationStrategyPojoStandard.java:154)
	at [email protected]//org.hibernate.metamodel.internal.ManagedTypeRepresentationResolverStandard.resolveStrategy(ManagedTypeRepresentationResolverStandard.java:62)
	at [email protected]//org.hibernate.persister.entity.AbstractEntityPersister.<init>(AbstractEntityPersister.java:562)
	at [email protected]//org.hibernate.persister.entity.SingleTableEntityPersister.<init>(SingleTableEntityPersister.java:134)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:481)
	at [email protected]//org.hibernate.persister.internal.PersisterFactoryImpl.createEntityPersister(PersisterFactoryImpl.java:94)
	... 19 more
Caused by: java.lang.NoSuchFieldException: invocationTarget$n2rb551
	at java.base/java.lang.Class.getDeclaredFieldInternal(Class.java:955)
	at java.base/java.lang.Class.getDeclaredField(Class.java:930)
	at [email protected]//net.bytebuddy.implementation.LoadedTypeInitializer$ForStaticField.onLoad(LoadedTypeInitializer.java:163)
	... 35 more

This is because we try to define/load the class org.jboss.as.test.integration.jpa.transaction.Company$HibernateAccessOptimizer5employees concurrently from N threads, though I believe N=3. The DynamicType building of that class uses intercept( MethodCall.call( Callable ) ), which leads to a randomly named field being added to the bytecode, that ByteBuddy initializes to the actual Callable instance after class loading.

It seems to me that DynamicType.Unloaded.load() might succeed in multiple threads, leading to ByteBuddy trying to initialize a field that doesn't exist. I would rather expect to see a LinkageError for every class definition call, other than the first one that enters the "critical section" in the JVM.

Any idea where this could go wrong? Also see our discussions on the Hibernate Zulip topic.

beikov avatar Jun 17 '25 12:06 beikov

If you create two classes with the same name multiple times, then loading it will cause the previously loaded class to be resolved. If the names of the fields are random (this can be changed, then that would be a nop), the loaded type initializer will fail as the class was already loaded with a different field name.

Byte Buddy does not know about the circumstances of the duplicated class, so failing is the right behaviour. I would suggest that Hibernate should create deterministic field names here, then the second initialization would not cause a problem. Alternatively, maybe there should be a check if a class was loaded previously with a lock on the name, or a catch block that suppresses that exception.

raphw avatar Jun 19 '25 06:06 raphw

Hi Rafael and thanks for the response. I understand that a deterministic name would be better and in fact, Hibernate doesn't even need the field at all, as we can encode the data/logic in bytecode. I wanted to bring this to your attention anyway, because AFAIU, there is a potential for a data race which is IMO non-obvious, but I need a little help from your side to fully understand what is going on exactly.

If you create two classes with the same name multiple times, then loading it will cause the previously loaded class to be resolved.

I guess that the term "loading" refers to the DynamicType.Unloaded.load() call? I would have expected that if two threads try to "load" or rather "define" a class of the same name at the same time, one of the two would fail with a LinkageError, but it seems to me that both "succeed" somehow, though obviously one fails later when trying to set the static field. We try to catch the LinkageError and simply return the previously defined class, but since a NoSuchFieldException is thrown, this doesn't work.

At this point, I'm trying to understand if there is a potential data race in ByteBuddy, because of the way parallel capable class loaders behave. AFAIU, the Lookup.defineClass method (which is used by ByteBuddy) delegates to the ClassLoader.defineClass method. My intuition is if two threads call defineClass concurrently, that one fails with a LinkageError, though I have not verified this yet. Maybe you know? Also, does ByteBuddy handle that somewhere and attempts to load the class then? If so, that would explain the exception that we're seeing right now and would confirm my hunch that there is a data race.

beikov avatar Jun 23 '25 08:06 beikov

The loading can be configured to fail if a class already exists: https://github.com/raphw/byte-buddy/blob/master/byte-buddy-dep/src/main/java/net/bytebuddy/dynamic/loading/ClassLoadingStrategy.java#L168

By default, Byte Buddy assumes that a class with the same name is already the same class, but the initialization fails if the class is different anyways. This can be suppressed without any damages, but this is left to the user.

Am Mo., 23. Juni 2025 um 10:03 Uhr schrieb Christian Beikov < @.***>:

beikov left a comment (raphw/byte-buddy#1827) https://github.com/raphw/byte-buddy/issues/1827#issuecomment-2995371141

Hi Rafael and thanks for the response. I understand that a deterministic name would be better and in fact, Hibernate doesn't even need the field at all, as we can encode the data/logic in bytecode. I wanted to bring this to your attention anyway, because AFAIU, there is a potential for a data race which is IMO non-obvious, but I need a little help from your side to fully understand what is going on exactly.

If you create two classes with the same name multiple times, then loading it will cause the previously loaded class to be resolved.

I guess that the term "loading" refers to the DynamicType.Unloaded.load() call? I would have expected that if two threads try to "load" or rather "define" a class of the same name at the same time, one of the two would fail with a LinkageError, but it seems to me that both "succeed" somehow, though obviously one fails later when trying to set the static field. We try to catch the LinkageError and simply return the previously defined class, but since a NoSuchFieldException is thrown, this doesn't work.

At this point, I'm trying to understand if there is a potential data race in ByteBuddy, because of the way parallel capable class loaders behave. AFAIU, the Lookup.defineClass method (which is used by ByteBuddy) delegates to the ClassLoader.defineClass method. My intuition is if two threads call defineClass concurrently, that one fails with a LinkageError, though I have not verified this yet. Maybe you know? Also, does ByteBuddy handle that somewhere and attempts to load the class then? If so, that would explain the exception that we're seeing right now and would confirm my hunch that there is a data race.

— Reply to this email directly, view it on GitHub https://github.com/raphw/byte-buddy/issues/1827#issuecomment-2995371141, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCIA4EJOZFVAVUZK6GIK3T3E6YGTAVCNFSM6AAAAAB7QEELH2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSOJVGM3TCMJUGE . You are receiving this because you were assigned.Message ID: @.***>

raphw avatar Jun 23 '25 09:06 raphw

I tried very hard to find a code path in the ByteBuddy types that are in use here which could lead to this, but was unable so far.

We use ClassLoadingStrategy.UsingLookup.of( MethodHandles.privateLookupIn( originalClass, LOOKUP ) ) as ClassLoadingStrategy, which AFAIU, does not do any fallback if the inject fails. Any idea if Lookup.defineClass() could succeed in two threads?

beikov avatar Jun 23 '25 10:06 beikov

Let's see what the OpenJ9 folks have to say. If this behavior turns out to be intentional and not in violation of the JVMS, it seems to me that ByteBuddy should maybe try to use the ClassLoader#getClassLoadingLock for OpenJ9 to avoid the problems that we have encountered.

beikov avatar Jun 23 '25 17:06 beikov

Byte Buddy is using the class loading lock when interacting with a class loader: https://github.com/raphw/byte-buddy/blob/master/byte-buddy-dep/src/main/java/net/bytebuddy/dynamic/loading/ClassInjector.java#L287

For lookups, this is not intended for the API, I was expecting the API to activate the underlying class loader locking, so this might just be a bug in OpenJ9.

raphw avatar Jun 25 '25 09:06 raphw