raphw / byte-buddy

Runtime code generation for the Java virtual machine.
https://bytebuddy.net
Apache License 2.0
6.22k stars 800 forks source link

A lazy `TypePool` will change with type resolution and thus deviate from its initial configuration. #1683

Open dogourd opened 1 month ago

dogourd commented 1 month ago
public class Test {

    interface Foo {
    }

    class Bar implements Foo {
    }

    public static void main(String[] args) {
        ClassLoader classLoader = ClassLoader.getSystemClassLoader();

        TypePool systemTypePool = TypePool.Default.of(classLoader);
        TypePool lazyResolution = new TypePool.Default.WithLazyResolution(
                TypePool.CacheProvider.NoOp.INSTANCE,
                ClassFileLocator.ForClassLoader.of(classLoader),
                TypePool.Default.ReaderMode.FAST,
                systemTypePool
        );
        TypePool lazyFacade = new TypePool.LazyFacade(lazyResolution);

        // the bar holds the typePool: lazyResolution.
        TypeDescription bar = lazyFacade.describe(Bar.class.getName()).resolve();

        // the foo is resolved by `systemTypePool.describe().resolve()` instead of `lazyFacade.describe().resolve()`
        TypeList.Generic interfaces = bar.getInterfaces();

        for (TypeDescription.Generic item : interfaces) {
            item.asErasure();
        }
    }
}

In the above code, when using the standard API to resolve the interface list information of bar, the initial LazyFacade is not attempted. In some scenarios, this is unexpected for me, causing certain logic that could have been skipped based on name checks to ultimately be interrupted by a NoSuchTypeException.

Specifically, some runtime types are modified by certain javaagents and implement specific interfaces within the agent. These types then cannot be resolved in my own agent, causing exceptions during type hierarchy traversal when using ElementMatchers.hasSuperType.

It's also worth mentioning that when there are unknown types in the interface implementation list, the iterator traversal form will encounter exceptions. For instance, if we assume that bar.getInterfaces() contains types not visible to the current TypePool, the following code will throw an exception (NoSuchTypeException). However, if traversed by index as shown below, no exception will occur:

for (TypeDescription.Generic item : interfaces) {
    // Potentially throws NoSuchTypeException
}

versus:

for (int i = 0; i < interfaces.size(); i++) {
    // working well
    TypeDescription.Generic item = interfaces.get(i);
}
raphw commented 1 month ago

Not sure I follow and I cannot reproduce the behaviour bellow. Can you create a reproducer project that demonstrates this? It is fully possible that I overlooked a lazy resolution somewhere. If you mix a lazy pool and an eager pool, as it seems to be the case however, there are no guarantees given.

dogourd commented 1 month ago

In the process of reproducing the issue, I observed further and found that it might be related to type caching operations.

During the WithLazyResolution.doResolve call, if it encounters an invisible type, it ultimately registers an Illegal object with the CacheProvider. This causes types that are invisible to WithLazyResolution to only provide the target type's name information during the first call to TypePool.describe. Subsequent calls will fail to successfully return a LazyResolution object because they cache the Illegal object, preventing the retrieval of the type's name information in later processes.

String dummy = "java.lang.Dummy";
TypePool.Default.WithLazyResolution lazyResolution = new TypePool.Default.WithLazyResolution(
         new TypePool.CacheProvider.Simple(),
         ClassFileLocator.NoOp.INSTANCE,
         TypePool.Default.ReaderMode.FAST
);

TypePool.Resolution resolution = lazyResolution.describe(dummy);
// print java.lang.Dummy
System.out.println(resolution.resolve().getName());

// do resolve.
resolution.isResolved();

// error.
lazyResolution.describe(dummy).resolve().getName();

Maybe I can extend the WithLazyResolution class and simply override its doResolve method to avoid registering the Illegal object, like this:

protected Resolution doResolve(String name) {
   Resolution resolution = super.doResolve(name);
   if (!resolution.isResolved()) {
        Resolution describe = doDescribe(name);
        resolution = new Resolution() {
             @Override
             public boolean isResolved() {
                return false;
             }
             @Override
             public TypeDescription resolve() {
                return describe.resolve();
             }
        };
        cacheProvider.register(name, resolution);
  }
  return resolution;
}

This way, I just need to ensure that the CacheProvider avoids using implementations like CacheProvider.Simple that use the putIfAbsent functionality.

Do you think this behavior is something the Byte-Buddy itself needs to consider?