ClassTransformer enhancement is not thread safe

Description

The classFileLocator in the EnhancerImpl seems non-thread safe, so this can cause issues that we should fix.

Activity

Show:

Scott Marlow September 21, 2023 at 6:23 PM

Thanks , I tried a local hack which seemed to help my testing:

```

diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java
index b6209fa918..eb09d2ce63 100644
--- a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java
+++ b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java
@@ -145,7 +145,9 @@ public class EnhancerImpl implements Enhancer {
       public byte[] enhance(String className, byte[] originalBytes) throws EnhancementException {
               //Classpool#describe does not accept '/' in the description name as it expects a class name. See HHH-12545
               final String safeClassName = className.replace( '/', '.' );
-               classFileLocator.setClassNameAndBytes( safeClassName, originalBytes );
+               synchronized (classFileLocator) {
+                       classFileLocator.setClassNameAndBytes( safeClassName, originalBytes );
+               }
               try {
                       final TypeDescription typeDescription = typePool.describe( safeClassName ).resolve();
 
@@ -165,7 +167,9 @@ public class EnhancerImpl implements Enhancer {
       @Override
       public void discoverTypes(String className, byte[] originalBytes) {
               if ( originalBytes != null ) {
-                       classFileLocator.setClassNameAndBytes( className, originalBytes );
+                       synchronized (classFileLocator) {
+                               classFileLocator.setClassNameAndBytes( className, originalBytes );
+                       }
               }
               try {
                       final TypeDescription typeDescription = typePool.describe( className ).resolve();
@@ -178,7 +182,9 @@ public class EnhancerImpl implements Enhancer {
       }
 
       private TypePool buildTypePool(final ClassFileLocator classFileLocator) {
-               return TypePool.Default.WithLazyResolution.of( classFileLocator );
+               synchronized (classFileLocator) {
+                       return TypePool.Default.WithLazyResolution.of( classFileLocator );
+               }
       }
 
       private DynamicType.Builder<?> doEnhance(DynamicType.Builder<?> builder, TypeDescription managedCtClass) {
```

With the above change, I can run a wildfly test now that uses bytecode enhancement and the test passes. I’m mentioning to confirm that the classFileLocator does need to be guarded.

Fixed

Details

Assignee

Reporter

Sprint

Fix versions

Affects versions

Priority

Created September 21, 2023 at 4:18 PM
Updated November 23, 2023 at 2:48 PM
Resolved September 22, 2023 at 4:05 PM