Skip to content

Conversation

timtebeek
Copy link
Member

@timtebeek timtebeek commented Sep 23, 2025

What's your motivation?

We've had reports in #212 that parallel and partial runs throw an exception when trying to load duplicate classes.

Have you considered any alternatives or workarounds?

  1. The docs on untracked are not a 100% clear on if this avoids all parallel runs.
  2. There's an alternative in getOutputs().upToDateWhen(task -> false); which we could set in AbstractRewriteTask instead.
  3. With more effort we might be able to tweak DelegatingProjectParser to support parallel classloaders each with their own GradleProjectParser, but the impact of that on reliable recipe runs is hard to judge at this moment, but likely leads to fragile runs.
  4. We could try to avoid the duplicate class loading by adding locks on org.openrewrite.gradle.RewriteClassLoader#loadClass, but it's not clear whether that's a viable option here, and again could lead to fragile recipe runs.

Any additional context

@timtebeek timtebeek self-assigned this Sep 23, 2025
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Sep 23, 2025
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Sep 23, 2025
@timtebeek timtebeek added the documentation Improvements or additions to documentation label Sep 23, 2025
@shanman190
Copy link
Collaborator

@timtebeek, sadly this doesn't disable parallel.

@UntrackedTask indicates that there shoudn't be any optimizations reliant upon input/output calculations (incremental, build cache, etc).

@timtebeek timtebeek marked this pull request as draft September 23, 2025 18:10
@timtebeek
Copy link
Member Author

Was already suspicious; who knew AI assistants could lie? 🙃

Among the other options there's also apparently something called shared build services, but that appears to be from 6.1 onwards, which isn't great with our focus to support back to 4.10. Any other suggestions perhaps?

@shanman190
Copy link
Collaborator

Java, by default, expects for loadClass to be thread safe. Ironically, even the parent implementation is thread safe, but our child implementation is not. All of the necessary plumbing is accessible to us though in order to make it thread safe, so I think we should go ahead and do that to resolve these parallel loading issues once and for all.

@timtebeek timtebeek changed the title Mark run tasks as untracked to avoid parallel runs Add synchronized (getClassLoadingLock(name)) in loadClass Sep 23, 2025
@timtebeek timtebeek changed the title Add synchronized (getClassLoadingLock(name)) in loadClass Remove unused methods from AbstractRewriteTask and RewriteClassLoader Sep 23, 2025
@timtebeek
Copy link
Member Author

Ok I had intended to push your suggestion to this branch, but instead I had switched back to main before 8942452. 😞

Updated the PR description for the small adjustments we're left with here, which are now very much optional to get in.

@timtebeek timtebeek marked this pull request as ready for review September 23, 2025 20:48
@shanman190 shanman190 merged commit 651a11a into main Sep 23, 2025
1 check passed
@shanman190 shanman190 deleted the mark-run-tasks-as-untracked branch September 23, 2025 20:51
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

loader org.openrewrite.gradle.RewriteClassLoader attempted duplicate class definition
2 participants