search

openrewrite / rewrite-maven-plugin

OpenRewrite's Maven plugin.
https://openrewrite.github.io/rewrite-maven-plugin/plugin-info.html
Apache License 2.0
140 stars 73 forks source link

`ConcurrentModificationException` while running `dryRun` with `mvnd` #780

Open motlin opened 5 months ago

motlin commented 5 months ago

What version of OpenRewrite are you using?

I am using

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a multi module project.

The command I ran was mvnd install org.openrewrite.maven:rewrite-maven-plugin:dryRun --projects '!liftwizard-example' --activate-profiles 'rewrite-maven-plugin-strict,rewrite-maven-plugin-dryRun' -DskipTests

What is the full stack trace of any errors you encountered?

Execution default-cli of goal org.openrewrite.maven:rewrite-maven-plugin:5.28.0:dryRun failed.
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:333)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at io.takari.maven.builder.smart.SmartBuilderImpl.buildProject (SmartBuilderImpl.java:209)
    at io.takari.maven.builder.smart.SmartBuilderImpl$ProjectBuildTask.run (SmartBuilderImpl.java:81)
    at java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:572)
    at java.util.concurrent.FutureTask.run (FutureTask.java:317)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1144)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:642)
    at java.lang.Thread.run (Thread.java:1570)
Caused by: org.apache.maven.plugin.PluginExecutionException: Execution default-cli of goal org.openrewrite.maven:rewrite-maven-plugin:5.28.0:dryRun failed.
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:133)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at io.takari.maven.builder.smart.SmartBuilderImpl.buildProject (SmartBuilderImpl.java:209)
    at io.takari.maven.builder.smart.SmartBuilderImpl$ProjectBuildTask.run (SmartBuilderImpl.java:81)
    at java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:572)
    at java.util.concurrent.FutureTask.run (FutureTask.java:317)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1144)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:642)
    at java.lang.Thread.run (Thread.java:1570)
Caused by: java.util.ConcurrentModificationException
    at java.util.LinkedHashMap$LinkedHashIterator.nextNode (LinkedHashMap.java:1023)
    at java.util.LinkedHashMap$LinkedKeyIterator.next (LinkedHashMap.java:1046)
    at org.apache.maven.project.MavenProject.getCompileClasspathElements (MavenProject.java:323)
    at org.openrewrite.maven.MavenMojoProjectParser.processMainSources (MavenMojoProjectParser.java:404)
    at org.openrewrite.maven.MavenMojoProjectParser.listSourceFiles (MavenMojoProjectParser.java:182)
    at org.openrewrite.maven.MavenMojoProjectParser.lambda$listSourceFiles$0 (MavenMojoProjectParser.java:154)
    at java.util.stream.ReferencePipeline$7$1.accept (ReferencePipeline.java:288)
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining (ArrayList.java:1709)
    at java.util.stream.AbstractPipeline.copyInto (AbstractPipeline.java:556)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto (AbstractPipeline.java:546)
    at java.util.stream.ReduceOps$ReduceOp.evaluateSequential (ReduceOps.java:921)
    at java.util.stream.AbstractPipeline.evaluate (AbstractPipeline.java:265)
    at java.util.stream.ReferencePipeline.collect (ReferencePipeline.java:702)
    at org.openrewrite.maven.AbstractRewriteMojo.sourcesWithAutoDetectedStyles (AbstractRewriteMojo.java:290)
    at org.openrewrite.maven.AbstractRewriteMojo.loadSourceSet (AbstractRewriteMojo.java:259)
    at org.openrewrite.maven.AbstractRewriteMojo.listResults (AbstractRewriteMojo.java:240)
    at org.openrewrite.maven.AbstractRewriteDryRunMojo.execute (AbstractRewriteDryRunMojo.java:68)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at io.takari.maven.builder.smart.SmartBuilderImpl.buildProject (SmartBuilderImpl.java:209)
    at io.takari.maven.builder.smart.SmartBuilderImpl$ProjectBuildTask.run (SmartBuilderImpl.java:81)
    at java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:572)
    at java.util.concurrent.FutureTask.run (FutureTask.java:317)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1144)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:642)
    at java.lang.Thread.run (Thread.java:1570)
shanman190 commented 5 months ago

Hi @motlin!

In your project are you running multiple modules/plugins in parallel? There's no threading in the OpenRewrite Maven build plugin, but it looks like the Maven project's resources are getting modified out from under the plugin itself based on the stacktrace.

motlin commented 5 months ago

Hi @shanman190, yes I'm running mvnd which is pretty similar to running mvn --threads 1C.

The rewrite plugin is explicitly marked thread-safe: https://github.com/search?q=repo%3Aopenrewrite%2Frewrite-maven-plugin%20threadSafe&type=code

timtebeek commented 5 months ago

Oh wow, that's quite unfortunate; I must admit I hadn't tried mvnd with OpenRewrite myself yet (the luxury of having Moderne). Definitely something to fix, or at worst mark the goals as not threadsafe for now. Any suggestions from your side? I'm just catching up and you might have learned more since.

timtebeek commented 5 months ago

In clicking through the stack trace I'm a bit puzzled why our use of getCompileClasspathElements() here would lead to a ConcurrentModificationException. https://github.com/openrewrite/rewrite-maven-plugin/blob/a482eff78c7bcd86ca5475e9a3f58e090f8e9841/src/main/java/org/openrewrite/maven/MavenMojoProjectParser.java#L404-L407

I don't think there's very much we could do differently, seeing how the CCME is triggered on this line in Apache Maven: https://github.com/apache/maven/blob/bc0240f3c744dd6b6ec2920b3cd08dcc295161ae/maven-core/src/main/java/org/apache/maven/project/MavenProject.java#L323

Does the same work for you if you use mvn --threads 1C instead of mvnd? I'm wondering if this is something to document, or if we should not mark our goals as thread-safe.

timtebeek commented 5 months ago

The getArtifacts() method called in Mavne does indicate that it's lazily populated

All dependencies that this project has, including transitive ones. Contents are lazily populated, so depending on what phases have run dependencies in some scopes won't be included. eg. if only compile phase has run,

Still not sure if that's something we'd have much control over. 🤔

shanman190 commented 5 months ago

I would think the phases would happen in a serial fashion within each module independently as that particular module works toward the specified goal.

To me this feels like a plugin that is modifying the current module's configuration within a separate thread or an entirely separate plugin in some other module running at the same time is crossing the module boundary. But I don't know for sure.

From the Maven docs, they state that the entirety of Maven proper is all thread safe, but they can make no guarantees on plugins and their specific behavior.

motlin commented 4 months ago

Because of the nature of this as a concurrency problem, I don't run into it frequently and haven't reproduced it with mvn --threads but they are extremely similar and I assume this is not a problem specific to mvnd.

It's always possible to make a maven plugin thread-safe but it would be difficult to do it reliably without understanding the problem better. For example, we could add synchronized blocks around the use of the mavenProject but it's a total guess at this point.

shanman190 commented 4 months ago

I think the problem here is OpenRewrite + something else. I'm not sure what the something else part of the equation is though. From an OpenRewrite standpoint, we are reading the dependencies -- as noted by Tim -- while something else is modifying the module's dependencies. Given Maven proper is all thread safe, my only remaining idea is some other plugin of either the current module or a plugin somewhere else in the Maven build is the one that is violating the thread safety here.