Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition triggered in java-generator for very big schemas #4723

Closed
andreaTP opened this issue Jan 3, 2023 · 20 comments · Fixed by #4849
Closed

Race condition triggered in java-generator for very big schemas #4723

andreaTP opened this issue Jan 3, 2023 · 20 comments · Fixed by #4849

Comments

@andreaTP
Copy link
Member

andreaTP commented Jan 3, 2023

Describe the bug

Using different versions of Java produce different errors with the java-generator

Fabric8 Kubernetes Client version

SNAPSHOT

Steps to reproduce

  • curl -Ls https://raw.githubusercontent.com/istio/istio/master/manifests/charts/base/crds/crd-all.gen.yaml -o istio.yaml
  • sdk use java 11.0.17-tem
  • jbang io.fabric8:java-generator-cli:6.3.1 --add-extra-annotations --source istio.yaml --target ./src

Produces a stack-trace like:

Exception in thread "main" java.lang.AssertionError
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
        at java.base/java.util.concurrent.ForkJoinTask.getThrowableException(ForkJoinTask.java:603)
        at java.base/java.util.concurrent.ForkJoinTask.reportException(ForkJoinTask.java:678)
        at java.base/java.util.concurrent.ForkJoinTask.invoke(ForkJoinTask.java:737)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateParallel(ForEachOps.java:159)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateParallel(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:233)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
        at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:661)
        at io.fabric8.java.generator.CRGeneratorRunner.runOnSingleSource(CRGeneratorRunner.java:82)
        at io.fabric8.java.generator.CRGeneratorRunner.run(CRGeneratorRunner.java:66)
        at io.fabric8.java.generator.cli.GenerateJavaSources.run(GenerateJavaSources.java:77)
        at picocli.CommandLine.executeUserObject(CommandLine.java:2026)
        at picocli.CommandLine.executeHelpRequest(CommandLine.java:2012)
        at picocli.CommandLine.executeHelpRequest(CommandLine.java:1983)
        at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2268)
        at picocli.CommandLine$RunLast.execute(CommandLine.java:2417)
        at picocli.CommandLine.execute(CommandLine.java:2170)
        at io.fabric8.java.generator.cli.GenerateJavaSources.main(GenerateJavaSources.java:81)
Caused by: java.lang.AssertionError
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
        at java.base/java.util.concurrent.ForkJoinTask.getThrowableException(ForkJoinTask.java:603)
        at java.base/java.util.concurrent.ForkJoinTask.reportException(ForkJoinTask.java:678)
        at java.base/java.util.concurrent.ForkJoinTask.invoke(ForkJoinTask.java:737)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateParallel(ForEachOps.java:159)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateParallel(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:233)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
        at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:661)
        at io.fabric8.java.generator.CRGeneratorRunner.lambda$runOnSingleSource$2(CRGeneratorRunner.java:93)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.ForEachOps$ForEachTask.compute(ForEachOps.java:290)
        at java.base/java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:746)
        at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
        at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
        at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
        at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
        at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)
Caused by: java.lang.AssertionError: I am not a child of my parent.
        at com.github.javaparser.printer.DefaultPrettyPrinterVisitor.printOrphanCommentsBeforeThisChildNode(DefaultPrettyPrinterVisitor.java:1799)
        at com.github.javaparser.printer.DefaultPrettyPrinterVisitor.visit(DefaultPrettyPrinterVisitor.java:1584)
        at com.github.javaparser.printer.DefaultPrettyPrinterVisitor.visit(DefaultPrettyPrinterVisitor.java:53)
        at com.github.javaparser.ast.expr.NormalAnnotationExpr.accept(NormalAnnotationExpr.java:74)
        at com.github.javaparser.printer.DefaultPrettyPrinterVisitor.printMemberAnnotations(DefaultPrettyPrinterVisitor.java:94)
        at com.github.javaparser.printer.DefaultPrettyPrinterVisitor.visit(DefaultPrettyPrinterVisitor.java:268)
        at com.github.javaparser.printer.DefaultPrettyPrinterVisitor.visit(DefaultPrettyPrinterVisitor.java:53)
        at com.github.javaparser.ast.body.ClassOrInterfaceDeclaration.accept(ClassOrInterfaceDeclaration.java:98)
        at com.github.javaparser.printer.DefaultPrettyPrinterVisitor.visit(DefaultPrettyPrinterVisitor.java:215)
        at com.github.javaparser.printer.DefaultPrettyPrinterVisitor.visit(DefaultPrettyPrinterVisitor.java:53)
        at com.github.javaparser.ast.CompilationUnit.accept(CompilationUnit.java:133)
        at com.github.javaparser.printer.DefaultPrettyPrinter.print(DefaultPrettyPrinter.java:95)
        at com.github.javaparser.ast.Node.toString(Node.java:322)
        at io.fabric8.java.generator.WritableCRCompilationUnit.writeAllJavaClasses(WritableCRCompilationUnit.java:58)
        at io.fabric8.java.generator.CRGeneratorRunner.lambda$runOnSingleSource$1(CRGeneratorRunner.java:93)
        ... 10 more

Additional issues can be experienced by mixing and matching Java and kubernetes-client versions, another example stack trace is:

java.lang.ArrayIndexOutOfBoundsException
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
        at java.base/java.util.concurrent.ForkJoinTask.getThrowableException(ForkJoinTask.java:603)
        at java.base/java.util.concurrent.ForkJoinTask.reportException(ForkJoinTask.java:678)
        at java.base/java.util.concurrent.ForkJoinTask.invoke(ForkJoinTask.java:737)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateParallel(ForEachOps.java:159)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateParallel(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:233)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
        at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:661)
        at io.fabric8.java.generator.FileJavaGenerator.runOnSingleSource(FileJavaGenerator.java:87)
        at io.fabric8.java.generator.FileJavaGenerator.run(FileJavaGenerator.java:71)
        at io.fabric8.java.generator.cli.GenerateJavaSources.lambda$run$0(GenerateJavaSources.java:132)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
        at io.fabric8.java.generator.cli.GenerateJavaSources.run(GenerateJavaSources.java:132)
        at picocli.CommandLine.executeUserObject(CommandLine.java:2026)
        at picocli.CommandLine.executeHelpRequest(CommandLine.java:2012)
        at picocli.CommandLine.executeHelpRequest(CommandLine.java:1983)
        at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2268)
        at picocli.CommandLine$RunLast.execute(CommandLine.java:2417)
        at picocli.CommandLine.execute(CommandLine.java:2170)
        at io.fabric8.java.generator.cli.GenerateJavaSources.main(GenerateJavaSources.java:136)
Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 8 out of bounds for length 7
        at java.base/java.util.ArrayList.add(ArrayList.java:487)
        at java.base/java.util.ArrayList.add(ArrayList.java:499)
        at com.github.javaparser.ast.Node.setParentNode(Node.java:435)
        at com.github.javaparser.ast.NodeList.setAsParentNodeOf(NodeList.java:559)
        at com.github.javaparser.ast.NodeList.own(NodeList.java:81)
        at com.github.javaparser.ast.NodeList.add(NodeList.java:73)
        at com.github.javaparser.ast.nodeTypes.NodeWithAnnotations.addAnnotation(NodeWithAnnotations.java:59)
        at io.fabric8.java.generator.nodes.JObjectExtraAnnotations.addExtraAnnotations(JObjectExtraAnnotations.java:36)
        at io.fabric8.java.generator.nodes.JObject.generateJava(JObject.java:156)
        at io.fabric8.java.generator.nodes.JObject.generateJava(JObject.java:168)
        at io.fabric8.java.generator.nodes.JArray.generateJava(JArray.java:51)
        at io.fabric8.java.generator.nodes.JObject.generateJava(JObject.java:168)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
        at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177)
        at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
        at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
        at io.fabric8.java.generator.CRGeneratorRunner.validateAndAggregate(CRGeneratorRunner.java:112)
        at io.fabric8.java.generator.CRGeneratorRunner.generate(CRGeneratorRunner.java:97)
        at io.fabric8.java.generator.FileJavaGenerator.lambda$runOnSingleSource$3(FileJavaGenerator.java:95)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.ForEachOps$ForEachTask.compute(ForEachOps.java:290)
        at java.base/java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:746)
        at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
        at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
        at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
        at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
        at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

Expected behavior

The generator correctly run and generate the sources.

Runtime

other (please specify in additional context)

Kubernetes API Server version

1.25.3@latest

Environment

Windows, Linux, macOS

Fabric8 Kubernetes Client Logs

No response

Additional context

No response

@andreaTP
Copy link
Member Author

andreaTP commented Jan 3, 2023

I'm completely puzzled by those errors, let me try to summon a couple of javaparser maintainers for a clue on where to start debugging here 🙏 cc. @jlerbsc @ftomassetti

@jlerbsc
Copy link

jlerbsc commented Jan 4, 2023

Hi @andreaTP What version of JavaParser are you using?

@jlerbsc
Copy link

jlerbsc commented Jan 4, 2023

The 2 stacktraces seem to correspond to processing on the annotations. So I tried to produce, with JP 3.24.10, a simple test case with one of the code sections that seems to cause the error.

io.fabric8.java.generator.nodes.JObjectExtraAnnotations.addExtraAnnotations(JObjectExtraAnnotations.java:36)

@Test
void issue4723() {
	String code = "class Foo {}";
	CompilationUnit cu = StaticJavaParser.parse(code);
	ClassOrInterfaceDeclaration cid = cu.findAll(ClassOrInterfaceDeclaration.class).get(0);
	addExtraAnnotations(cid);
	System.out.println(cu.toString());
}

void addExtraAnnotations(ClassOrInterfaceDeclaration clz) {
    clz.addAnnotation("lombok.ToString");
    if (clz.getExtendedTypes().isEmpty()) {
      clz.addAnnotation("lombok.EqualsAndHashCode");
    } else {
      clz.addAnnotation(new SingleMemberAnnotationExpr(
          new Name("lombok.EqualsAndHashCode"),
          new NameExpr("callSuper = true")));
    }
    clz.addAnnotation("lombok.Setter");

    clz.addAnnotation(
        new SingleMemberAnnotationExpr(
            new Name("lombok.experimental.Accessors"),
            new NameExpr("prefix = {\n" + "    \"_\",\n" + "    \"\"\n" + "}")));

    clz.addAnnotation(
        new SingleMemberAnnotationExpr(
            new Name("io.sundr.builder.annotations.Buildable"),
            new NameExpr(
                "editableEnabled = false, validationEnabled = false, generateBuilderPackage = false, builderPackage = \"io.fabric8.kubernetes.api.builder\", refs = {\n"
                    + "    @io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.ObjectMeta.class),\n"
                    + "    @io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.ObjectReference.class),\n"
                    + "    @io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.LabelSelector.class),\n"
                    + "    @io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.Container.class),\n"
                    + "    @io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.EnvVar.class),\n"
                    + "    @io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.ContainerPort.class),\n"
                    + "    @io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.Volume.class),\n"
                    + "    @io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.VolumeMount.class)\n"
                    + "}")));
  }

Here is the produced result

@lombok.ToString()
@lombok.EqualsAndHashCode()
@lombok.Setter()
@lombok.experimental.Accessors(prefix = {
"_",
""
})
@io.sundr.builder.annotations.Buildable(editableEnabled = false, validationEnabled = false, generateBuilderPackage = false, builderPackage = "io.fabric8.kubernetes.api.builder", refs = {
@io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.ObjectMeta.class),
@io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.ObjectReference.class),
@io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.LabelSelector.class),
@io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.Container.class),
@io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.EnvVar.class),
@io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.ContainerPort.class),
@io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.Volume.class),
@io.sundr.builder.annotations.BuildableReference(io.fabric8.kubernetes.api.model.VolumeMount.class)
})
class Foo {
}

Does this correspond to what you want to generate?

@andreaTP
Copy link
Member Author

andreaTP commented Jan 4, 2023

Hi @andreaTP What version of JavaParser are you using?

Hi @jlerbsc , thanks a lot for looking into this, much appreciated!

kubernetes-client version 6.3.1 -> javaparser 3.24.9
kubernetes-client version 6.4-SNAPSHOT -> javaparser 3.24.10

and yes, your output is very similar to the expected one ( e.g. is comparable with the approval tests output ).

@jlerbsc
Copy link

jlerbsc commented Jan 4, 2023

Are you modifying the AST with multiple threads?

@andreaTP
Copy link
Member Author

andreaTP commented Jan 4, 2023

No, we are processing multiple CRDs in parallel and we are parallelizing the write to disk, but the processing is executed on a single thread

@jlerbsc
Copy link

jlerbsc commented Jan 4, 2023

On which version of java are you getting this stacktrace?

Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 8 out of bounds for length 7
at java.base/java.util.ArrayList.add(ArrayList.java:487)
at java.base/java.util.ArrayList.add(ArrayList.java:499)
at com.github.javaparser.ast.Node.setParentNode(Node.java:435)

Ideally you should be able to isolate a simplified test case that reproduces the issue because otherwise it will be difficult to help you.

@andreaTP
Copy link
Member Author

andreaTP commented Jan 4, 2023

I completely understand, I will try to create a more minimal reproducer for the issue 👍 I was trying to see if the StackTrace itself rings any bell 🙂

@jlerbsc
Copy link

jlerbsc commented Jan 29, 2023

Were you able to reproduce your error with a simplified test case?

@andreaTP
Copy link
Member Author

andreaTP commented Feb 8, 2023

Hi @jlerbsc I'm so sorry for the time this is taking 😞
But I have started a separate thread here: #4849 to try to reproduce and minimize the issue.

Indeed the underlying problem seems to be a race of some kind, I change the title accordingly.

@andreaTP andreaTP changed the title Verify java cross compatibility of java-generator Race condition triggered in java-generator for very big schemas Feb 8, 2023
@jlerbsc
Copy link

jlerbsc commented Feb 8, 2023

Hi @andreaTP Maybe it is possible to try to synchronize JObject.generateJava method.

@andreaTP
Copy link
Member Author

andreaTP commented Feb 8, 2023

Thanks for the suggestion @jlerbsc ! I have done it(out of desperation) but it doesn't seem to have any impact.
An additional element is that running the same code path in the tests using the same input is correctly generating 1603 Java classes with a reliability of 100%.

I was analyzing the issue with @mariofusco (thanks again 🙏 ) today, and we are starting to believe that there is a shared element "somewhere" in the stack that is causing errors to happen randomly because of a race condition.

Please note that the error is not "stable" and it's happening at different depths of the AST and on different elements, excluding that the issue depends on the actual input.

@jlerbsc
Copy link

jlerbsc commented Feb 8, 2023

JP sometimes uses static methods, in particular the lexical preserving printer and the symbol solver, which is certainly the reason why the designers of this project have always proclaimed loud and clear that JP was not thead safe. There have been initiatives to try to improve things here and there, but there may still be some code that is not thread safe.

@andreaTP
Copy link
Member Author

andreaTP commented Feb 9, 2023

Thanks a lot for this honest explanation @jlerbsc and for staying involved in this issue/discussion, much appreciated!

Tomorrow both me and @mariofusco are probably going to spend a few more hours trying to narrow down this issue.
If nothing comes up my proposed action plan would be to:

This action is surely going to impact the performance, but, at this stage, we should favor the correctness and get back to performance when we see the need.

@andreaTP
Copy link
Member Author

andreaTP commented Feb 9, 2023

Thanks to @mariofusco that found out that the race condition is happening on this node incorrectly encoded:
https://github.com/fabric8io/kubernetes-client/pull/4849/files#diff-a297bc569285c2dbea4f4c5d02362681ee8ea3be3156d3df20ea6d71b3dbb712R45

Thanks again for the support to everyone involved!

@lmyslinski
Copy link

Hey, I've just ran into this - is there any workaround or snapshot version I could use until 6.5.0 is out?

@andreaTP
Copy link
Member Author

Hi @lmyslinski , I failed to lookup your account when I found out that the problem was reliably reproducible with you CRD.
You should be able to use 6.5-SNAPSHOT from the nightly builds, let me know if this works for you!

@lmyslinski
Copy link

lmyslinski commented Feb 15, 2023

Hi @lmyslinski , I failed to lookup your account when I found out that the problem was reliably reproducible with you CRD. You should be able to use 6.5-SNAPSHOT from the nightly builds, let me know if this works for you!

@andreaTP Hmm I'm afraid that the plugin is not pushed to snapshots:

Could not find artifact io.fabric8:java-generator-maven-plugin:jar:6.5-SNAPSHOT

I've added the following snapshot repo:

  <repositories>
    <repository>
      <id>ossrh</id>
      <name>Sonatype OSS Repository (Snapshots)</name>
      <url>https://oss.sonatype.org/content/repositories/snapshots</url>
      <snapshots>
        <enabled>true</enabled>
      </snapshots>
      <releases>
        <enabled>false</enabled>
      </releases>
    </repository>
  </repositories>

@lmyslinski
Copy link

I've figured it out, plugins have a separate repo scope:

  <pluginRepositories>
    <pluginRepository>
      <id>ossrh</id>
      <name>Sonatype OSS Repository (Snapshots)</name>
      <url>https://oss.sonatype.org/content/repositories/snapshots</url>
      <snapshots>
        <enabled>true</enabled>
      </snapshots>
      <releases>
        <enabled>false</enabled>
      </releases>
    </pluginRepository>
  </pluginRepositories>

All good now, thx a lot!

@andreaTP
Copy link
Member Author

@lmyslinski I was about to answer 🙂 but you already got it 👍 thanks for the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants