search

apache / incubator-kie-drools

Drools is a rule engine, DMN engine and complex event processing (CEP) engine for Java.
http://www.drools.org
5.85k stars 2.49k forks source link

ClassCastException in Executable model #5925

Closed flozano closed 4 months ago

flozano commented 4 months ago

I am trying to prepare a reproducing case for this. As it's a complex case and the java classes of the model being evaluated are runtime-generated by byte-buddy, it's not easy...

When loading an Excel decision table with executable model (or the equivalent generated DRL from the Excel decision table), we are getting this exception:

Caused by: java.lang.ClassCastException: class sun.reflect.generics.reflectiveObjects.TypeVariableImpl cannot be cast to class java.lang.Class (sun.reflect.generics.reflectiveObjects.TypeVariableImpl and java.lang.Class are in module java.base of loader 'bootstrap')
    at org.drools.model.codegen.execmodel.generator.drlxparse.SpecialComparisonCase.typeNeedsCast(SpecialComparisonCase.java:71)
    at org.drools.model.codegen.execmodel.generator.drlxparse.SpecialComparisonCase.specialComparisonFactory(SpecialComparisonCase.java:56)
    at org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.handleSpecialComparisonCases(ConstraintParser.java:900)
    at org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.parseBinaryExpr(ConstraintParser.java:712)
    at org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.compileToJavaRecursive(ConstraintParser.java:317)
    at org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.compileStart(ConstraintParser.java:269)
    at org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.drlxParse(ConstraintParser.java:164)
    at org.drools.model.codegen.execmodel.generator.visitor.pattern.ClassPatternDSL.findAllConstraint(ClassPatternDSL.java:188)
    at org.drools.model.codegen.execmodel.generator.visitor.pattern.PatternDSL.buildPattern(PatternDSL.java:62)
    at org.drools.model.codegen.execmodel.generator.visitor.ModelGeneratorVisitor.visit(ModelGeneratorVisitor.java:132)
    at org.drools.drl.ast.descr.PatternDescr.accept(PatternDescr.java:278)
    at org.drools.model.codegen.execmodel.generator.visitor.AndVisitor.visit(AndVisitor.java:52)
    at org.drools.model.codegen.execmodel.generator.visitor.ModelGeneratorVisitor.visit(ModelGeneratorVisitor.java:70)
    at org.drools.model.codegen.execmodel.generator.ModelGenerator.processRule(ModelGenerator.java:204)
    at org.drools.model.codegen.execmodel.generator.ModelGenerator.processRuleDescr(ModelGenerator.java:185)
    at org.drools.model.codegen.execmodel.generator.ModelGenerator.lambda$processRules$0(ModelGenerator.java:158)
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
    at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
    at java.base/java.util.stream.ForEachOps$ForEachTask.compute(ForEachOps.java:291)
    at java.base/java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:754)

I will update this issue with more details when I can reproduce isolated... but decided to report early as it may be useful already:

here

    private static Optional<Class<?>> typeNeedsCast(Type t) {
        boolean needCast = isObject((Class<?>)t) || isMap((Class<?>) t) || isList((Class<?>) t);
        if (needCast) {
            return of((Class<?>) t);
        } else {
            return Optional.empty();
        }
    }

and here

    static SpecialComparisonCase specialComparisonFactory(TypedExpression left, TypedExpression right) {
        if (isNumber(left) && !isObject(right.getRawClass()) || isNumber(right) && !isObject(left.getRawClass())) { // Don't coerce Object yet. EvaluationUtil will handle it dynamically later
            Optional<Class<?>> leftCast = typeNeedsCast(left.getType());
            Optional<Class<?>> rightCast = typeNeedsCast(right.getType());
            if (leftCast.isPresent() || rightCast.isPresent()) {
                return new ComparisonWithCast(true, left, right, of(Number.class), of(Number.class));
            } else {
                return new NumberComparisonWithoutCast(left, right);
            }
        }
        if (!isComparable(left.getRawClass()) && !isComparable(right.getRawClass())){
            return new ComparisonWithCast(left, right, of(Comparable.class), of(Comparable.class));
        }
        return new PlainEvaluation(left, right);
    }

it is assumed any TypedExpression refers to a Type that is a Class.

mariofusco commented 4 months ago

I (sort of) see what you mean and we probably cannot give for granted that the Type is always a Class in that point, but at the moment I don't understand under which conditions it could be something else, and I really need a reproducer of this problem since I don't want to just develop a blind fix. Can you please just paste here the drl of a sample rule in your knowledge base that is causing this issue so I could try to create the isolated reproducer on my own?

flozano commented 4 months ago

I understand not wanting to fix blindly unless the scenario is clear.

The DRL is this:

package com.mycompany.myproject.engine.rules.model;
//generated from Decision Table
import com.mycompany.myproject.engine.rules.model.*;
global com.mycompany.myproject.engine.rules.model.ServerCodeRuntimeService serverCode;
dialect "mvel"
// rule values at B13, header at B8
rule "Tank Rules-SmartBIN_13"
    salience 65535
    when
        $asset: Sbtanknew_1_0_6($asset.devices.tank_device.properties.tanklevel_status != "Nearing Empty", $asset.devices.tank_device.readings.tank_level < $asset.properties.empty_fill_level_threshold)
    then
        $asset.getDevices().getTank_device().getProperties().setTanklevel_status("Nearing Empty");
        $asset.getDevices().getTank_device().getProperties().setTanklevel_color("Red");
        $asset.getProperties().setTank_level_status("CRITICAL");
        AlertMessage message = new AlertMessage("Nearing Empty");
        message.withParam("value", $asset.devices.tank_device.readings.tank_level);
        message.withParam("alias", "sb_device_tank_level");
        message.withParam("field", "Tank Level");
        message.withParam("is_required", "Required");
        insert(message);
end

if I drop $asset.devices.tank_device.readings.tank_level < $asset.properties.empty_fill_level_threshold, the above error goes away and a different one appears, but this one I need to investigate further. It also seems related to the fact that a generic class is involved. (I understand $asset inside the condition is redundant).

the model classes (eg:Sbtanknew_1_0_6) are derived from base classes and are runtime generated with byte-buddy, overriding several generic parameters of the parent class. I think this is where the issue comes from, and why it's not easy to isolate. I will try.

flozano commented 4 months ago

I have the feeling this could be related: https://issues.redhat.com/browse/DROOLS-7197, which I reported long time ago and was assumed to become a known issue partially documented in https://docs.drools.org/latest/drools-docs/drools/migration-guide/index.html

flozano commented 4 months ago

This is my attempt at reproducing it but no luck so far: https://github.com/flozano/drools-issues/blob/main/src/test/java/drools/issues/DynamicClassTest.java

Classes are generated here dynamically in a similar way as I generate the ones causing trouble, with byte-buddy: https://github.com/flozano/drools-issues/blob/main/src/main/java/drools/issues/model/vehicles/GasolineModelGenerator.java

and this DRL works well over them: https://github.com/flozano/drools-issues/blob/main/src/test/resources/drools/issues/DynamicClassTest.drl

mariofusco commented 4 months ago

if I drop $asset.devices.tank_device.readings.tank_level < $asset.properties.empty_fill_level_threshold, the above error goes away and a different one appears, but this one I need to investigate further. It also seems related to the fact that a generic class is involved. (I understand $asset inside the condition is redundant).

Is there any reason why you're keeping that $asset inside the condition? Does it change anything if you remove it (I don't expect so, but just in case).

the model classes (eg:Sbtanknew_1_0_6) are derived from base classes and are runtime generated with byte-buddy, overriding several generic parameters of the parent class. I think this is where the issue comes from, and why it's not easy to isolate. I will try.

If the executable model generator can access the runtime classes generated by byte-buddy and introspect them during the code generation phase I believe that there shouldn't be any problem. My wild guess is that the executable model generator is introspecting the original classes before byte-buddy instrumentation and if so of course that's wrong and the root cause of the problem. Another (in my opinion more remote) possibility is that byte-buddy is rewriting the bytecode of the domain model classes in a way that for some reason prevents the introspection necessary for the executable model generation, but if so we would also need a reproducer for that.

flozano commented 4 months ago

Thanks for answering!

Is there any reason why you're keeping that $asset inside the condition? Does it change anything if you remove it (I don't expect so, but just in case).

I'm keeping because the original DRL had it and I wanted to reproduce exactly - but no, it doesn't change anything. I mentioned in:

(I understand $asset inside the condition is redundant).

With regard to:

If the executable model generator can access the runtime classes generated by byte-buddy and introspect them during the code generation phase I believe that there shouldn't be any problem.

I believe so too. I use the correct strategy in byte-buddy (ClassLoadingStrategy.Default.CHILD_FIRST_PERSISTENT) and expose the right class-loader to drools... so everything is there for drools to access.

update

I just reproduced it, you can find the failing version here: https://github.com/flozano/drools-issues/blob/main/src/test/java/drools/issues/DynamicClassTest.java

The changes to reproduce: After properly subclassing in byte-buddy with the generic type descriptor instead of the raw Vehicle class, and using the property getter defined in the parent item (motor instead of engine), I expected to find same issue as in #5932 (https://issues.redhat.com/browse/DROOLS-7197), but I didn't, I reproduced this.

full stack-trace

``` class sun.reflect.generics.reflectiveObjects.TypeVariableImpl cannot be cast to class java.lang.Class (sun.reflect.generics.reflectiveObjects.TypeVariableImpl and java.lang.Class are in module java.base of loader 'bootstrap') java.lang.ClassCastException: class sun.reflect.generics.reflectiveObjects.TypeVariableImpl cannot be cast to class java.lang.Class (sun.reflect.generics.reflectiveObjects.TypeVariableImpl and java.lang.Class are in module java.base of loader 'bootstrap') at org.drools.model.codegen.execmodel.generator.drlxparse.SpecialComparisonCase.typeNeedsCast(SpecialComparisonCase.java:71) at org.drools.model.codegen.execmodel.generator.drlxparse.SpecialComparisonCase.specialComparisonFactory(SpecialComparisonCase.java:56) at org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.handleSpecialComparisonCases(ConstraintParser.java:900) at org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.parseBinaryExpr(ConstraintParser.java:712) at org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.compileToJavaRecursive(ConstraintParser.java:317) at org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.compileStart(ConstraintParser.java:269) at org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.drlxParse(ConstraintParser.java:164) at org.drools.model.codegen.execmodel.generator.visitor.pattern.ClassPatternDSL.findAllConstraint(ClassPatternDSL.java:188) at org.drools.model.codegen.execmodel.generator.visitor.pattern.PatternDSL.buildPattern(PatternDSL.java:62) at org.drools.model.codegen.execmodel.generator.visitor.ModelGeneratorVisitor.visit(ModelGeneratorVisitor.java:132) at org.drools.drl.ast.descr.PatternDescr.accept(PatternDescr.java:278) at org.drools.model.codegen.execmodel.generator.visitor.AndVisitor.visit(AndVisitor.java:52) at org.drools.model.codegen.execmodel.generator.visitor.ModelGeneratorVisitor.visit(ModelGeneratorVisitor.java:70) at org.drools.model.codegen.execmodel.generator.ModelGenerator.processRule(ModelGenerator.java:204) at org.drools.model.codegen.execmodel.generator.ModelGenerator.processRuleDescr(ModelGenerator.java:185) at org.drools.model.codegen.execmodel.generator.ModelGenerator.processRules(ModelGenerator.java:164) at org.drools.model.codegen.execmodel.generator.ModelGenerator.generateModel(ModelGenerator.java:130) at org.drools.model.codegen.execmodel.processors.ModelGeneratorPhase.process(ModelGeneratorPhase.java:48) at org.drools.compiler.builder.impl.processors.IteratingPhase.process(IteratingPhase.java:53) at org.drools.model.codegen.execmodel.processors.ModelMainCompilationPhase.process(ModelMainCompilationPhase.java:104) at org.drools.model.codegen.execmodel.ModelBuilderImpl.doSecondBuildStep(ModelBuilderImpl.java:114) at org.drools.compiler.builder.impl.CompositeKnowledgeBuilderImpl.build(CompositeKnowledgeBuilderImpl.java:125) at org.drools.compiler.builder.impl.CompositeKnowledgeBuilderImpl.build(CompositeKnowledgeBuilderImpl.java:109) at org.drools.compiler.kie.builder.impl.AbstractKieProject.buildKnowledgePackages(AbstractKieProject.java:274) at org.drools.compiler.kie.builder.impl.AbstractKieProject.buildKnowledgePackages(AbstractKieProject.java:220) at org.drools.compiler.kie.builder.impl.AbstractKieProject.verify(AbstractKieProject.java:84) at org.drools.compiler.kie.builder.impl.KieBuilderImpl.buildKieProject(KieBuilderImpl.java:285) at org.drools.compiler.kie.builder.impl.KieBuilderImpl.buildAll(KieBuilderImpl.java:251) at org.drools.compiler.kie.builder.impl.KieBuilderImpl.buildAll(KieBuilderImpl.java:208) at drools.issues.executor.DroolsInternalFactory.instanceKieContainer(DroolsInternalFactory.java:52) at drools.issues.executor.AbstractDroolsExecutor.(AbstractDroolsExecutor.java:56) at drools.issues.executor.AbstractDroolsExecutor.(AbstractDroolsExecutor.java:45) at drools.issues.executor.SingleStatefulSessionExecutor.(SingleStatefulSessionExecutor.java:15) at drools.issues.AbstractTest.executor(AbstractTest.java:16) at drools.issues.DynamicClassTest.test(DynamicClassTest.java:39) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725) at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60) at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131) at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149) at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140) at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestTemplateMethod(TimeoutExtension.java:92) at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115) at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105) at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106) at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64) at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45) at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37) at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104) at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:214) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:210) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:66) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35) at org.junit.platform.engine.support.hierarchical.NodeTestTask$DefaultDynamicTestExecutor.execute(NodeTestTask.java:226) at org.junit.platform.engine.support.hierarchical.NodeTestTask$DefaultDynamicTestExecutor.execute(NodeTestTask.java:204) at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.execute(TestTemplateTestDescriptor.java:139) at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.lambda$execute$2(TestTemplateTestDescriptor.java:107) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133) at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1939) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.execute(TestTemplateTestDescriptor.java:107) at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.execute(TestTemplateTestDescriptor.java:42) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35) at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57) at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:107) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52) at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114) at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86) at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:119) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:94) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:89) at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:62) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33) at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94) at jdk.proxy1/jdk.proxy1.$Proxy2.stop(Unknown Source) at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193) at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60) at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:113) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:65) at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69) at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74) ```

mariofusco commented 4 months ago

I will try to give a look at that reproducer asap, but what I don't understand is why it is not possible to create the same reproducer in a more isolated way and without byte-buddy. I mean once you have done the byte-buddy instrumentation those generated classes should be indistinguishable from classes generated by a normal compilation. In essence what I'm suggesting is that it should be possible to also reproduce the same problem with a plain Java domain model that incorporates in its source the changes made at bytecode level by byte-buddy. Am I making sense? Do you think it would be possible to create such a reproducer?

flozano commented 4 months ago

I agree with you that it should be possible, but if you look at what my byte-buddy code does, it should be the same already...

Base

public abstract class Vehicle<TEngine extends Engine> {
    public abstract TEngine getEngine();

public abstract class Engine {
// ...

"Diesel" model, 'normal' java compilation-generated classes

public class DieselCar extends Vehicle<DieselEngine> {
// ...
    public DieselEngine getEngine() {
        return engine;
    }
// ...
public class DieselEngine extends Engine {

"Gasoline" model, byte-buddy generated classes

engineClass = new ByteBuddy()
    .subclass(Engine.class).name("drools.issues.model.vehicles.GasolineEngine") //
    .method(ElementMatchers.named("isZeroEmissions")) //
    .intercept(FixedValue.value(false)) //
    .defineProperty("highOctane", boolean.class) //
    .defineProperty("maxTorque", int.class) //
    .defineProperty("serialNumber", long.class) //
    .make().load(classLoader, ClassLoadingStrategy.Default.CHILD_FIRST_PERSISTENT).getLoaded();

TypeDescription.Generic generic = TypeDescription.Generic.Builder.parameterizedType(Vehicle.class, engineClass).build();
vehicleClass = (Class<? extends Vehicle>) new ByteBuddy() //
        // .subclass(Vehicle.class) //
        .subclass(generic)
        .name("drools.issues.model.vehicles.GasolineVehicle") //
        .defineField("engine", engineClass, Visibility.PRIVATE) //
        .defineConstructor(Visibility.PUBLIC) //
        .withParameters(String.class, String.class, engineClass) //
        .intercept(MethodCall.invoke(Vehicle.class.getDeclaredConstructor(String.class, String.class))
                .withArgument(0, 1) //
                .andThen(FieldAccessor.ofField("engine").setsArgumentAt(2)) //
        ) //
        .defineMethod("getEngine", engineClass, Visibility.PUBLIC) //
        .intercept(FieldAccessor.ofField("engine")) //
        .defineProperty("frameMaxTorque", long.class) //
        .make().load(classLoader, ClassLoadingStrategy.Default.CHILD_FIRST_PERSISTENT).getLoaded();

the byte-buddy generated class is legal JVM code that can be executed. I can dispatch invocations to parent class (Engine) methods, subclass (DieselEngine) methods... as per the JVM this should be legal code, I believe.

flozano commented 4 months ago

from byte-buddy, it very explicitly supports subclasses of generic parents:

    /**
     * <p>
     * Creates a new builder for subclassing the provided type. If the provided type is an interface, a new class implementing
     * this interface type is created.
     * </p>
     * <p>
     * When extending a class, Byte Buddy imitates all visible constructors of the subclassed type and sets them to be {@code public}.
     * Any constructor is implemented to only invoke its super type constructor of equal signature. Another behavior can be specified by
     * supplying an explicit {@link ConstructorStrategy} by {@link ByteBuddy#subclass(TypeDefinition, ConstructorStrategy)}.
     * </p>
     * <p>
     * <b>Note</b>: This methods implements the supplied types <i>as is</i>, i.e. any {@link TypeDescription} values are implemented
     * as raw types if they declare type variables.
     * </p>
     * <p>
     * <b>Note</b>: Byte Buddy does not cache previous subclasses but will attempt the generation of a new subclass. For caching
     * types, a external cache or {@link TypeCache} should be used.
     * </p>
     *
     * @param superType The super class or interface type to extend. The type must be a raw type or parameterized type. All type
     *                  variables that are referenced by the generic type must be declared by the generated subclass before creating
     *                  the type.
     * @return A type builder for creating a new class extending the provided class or interface.
     */
    public DynamicType.Builder<?> subclass(TypeDefinition superType) {
        return subclass(superType, ConstructorStrategy.Default.IMITATE_SUPER_CLASS_OPENING);
    }

superType The super class or interface type to extend. The type must be a raw type or parameterized type. All type variables that are referenced by the generic type must be declared by the generated subclass before creating the type.

flozano commented 4 months ago

(just pushed a small fix for another unrelated method which was declared package-private but public in the byte-buddy. it has no effect on this specific test and case).

flozano commented 4 months ago

@mariofusco after you pushed for checking this better, I verified I could also reproduce this with non-byte-buddy classes:

https://github.com/flozano/drools-issues/blob/main/src/test/java/drools/issues/DynamicClassTest.java#L26

I copied the test and replicated it with DieselCar/Engine, which are normal java classes.

mariofusco commented 4 months ago

Fixed by https://github.com/apache/incubator-kie-drools/commit/785ef86a9a774db20f8610a1b73f4d5cdb635eb4

flozano commented 4 months ago

Thanks a lot! I will verify once the SNAPSHOT is built.

The same test incidentally raised #5943, will check that one too.