Configuration#withDependencies can run after the configuration has been resolved

[isker]

Expected Behavior

Configuration#withDependencies should always run before the configuration is no longer valid to mutate. See its javadoc.

Current Behavior

In rare cases on our CI (less than 1% of build executions), we get an exception when a withDependencies closure mutates the DependencySet it is passed.

Unfortunately the build is closed-source and, as I can't even reproduce it outside of our CI environment, I have no reproducing example. Here is an outline of the failure.

In our plugin:

var configuration =  . . . 
configuration.withDependencies { deps ->
    // do some work to calculate a new dependency
    var newDependency = project.dependencies.create(...)
    deps.add(newDependency) // this throws some small fraction of the time
}

The relevant frames of the associated stacktrace from our CI logs:

09:25:07 Caused by: org.gradle.api.InvalidUserDataException: Cannot change dependencies of configuration 'myConfiguration' after it has been included in dependency resolution.
09:25:07     at org.gradle.api.internal.artifacts.configurations.DefaultConfiguration.preventIllegalMutation(DefaultConfiguration.java:1136)
09:25:07     at org.gradle.api.internal.artifacts.configurations.DefaultConfiguration.validateMutation(DefaultConfiguration.java:1098)
09:25:07     at org.gradle.api.internal.artifacts.configurations.DefaultConfiguration$3.execute(DefaultConfiguration.java:299)
09:25:07     at org.gradle.api.internal.artifacts.configurations.DefaultConfiguration$3.execute(DefaultConfiguration.java:296)
09:25:07     at org.gradle.internal.ImmutableActionSet$SingletonSet.execute(ImmutableActionSet.java:225)
09:25:07     at org.gradle.api.internal.DefaultDomainObjectSet.assertMutableCollectionContents(DefaultDomainObjectSet.java:65)
09:25:07     at org.gradle.api.internal.DefaultDomainObjectCollection.add(DefaultDomainObjectCollection.java:252)
09:25:07     at org.gradle.api.internal.DelegatingDomainObjectSet.add(DelegatingDomainObjectSet.java:110)
09:25:07     at org.gradle.api.internal.artifacts.DefaultDependencySet.add(DefaultDependencySet.java:64)
09:25:07     at org.gradle.api.internal.artifacts.DefaultDependencySet$add.call(Unknown Source)
09:25:07     at MyPlugin$_apply_closure2.doCall(MyPlugin.groovy:124)
09:25:07     at jdk.internal.reflect.GeneratedMethodAccessor563.invoke(Unknown Source)
09:25:07     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
09:25:07     at org.gradle.util.ClosureBackedAction.execute(ClosureBackedAction.java:71)
09:25:07     at org.gradle.util.ConfigureUtil.configureTarget(ConfigureUtil.java:154)
09:25:07     at org.gradle.util.ConfigureUtil.configure(ConfigureUtil.java:105)
09:25:07     at org.gradle.util.ConfigureUtil$WrappedConfigureAction.execute(ConfigureUtil.java:166)
09:25:07     at org.gradle.internal.ImmutableActionSet$SingletonSet.execute(ImmutableActionSet.java:225)
09:25:07     at org.gradle.api.internal.artifacts.configurations.DefaultConfiguration.runDependencyActions(DefaultConfiguration.java:463)
09:25:07     at org.gradle.api.internal.artifacts.configurations.DefaultConfiguration.runDependencyActions(DefaultConfiguration.java:470)
09:25:07     at org.gradle.api.internal.artifacts.configurations.DefaultConfiguration.runDependencyActions(DefaultConfiguration.java:470)
09:25:07     at org.gradle.internal.component.local.model.DefaultLocalComponentMetadata$DefaultLocalConfigurationMetadata.realizeDependencies(DefaultLocalComponentMetadata.java:511)
09:25:07     at org.gradle.internal.component.local.model.DefaultLocalComponentMetadata$DefaultLocalConfigurationMetadata.addDefinedExcludes(DefaultLocalComponentMetadata.java:469)
09:25:07     at org.gradle.internal.component.local.model.DefaultLocalComponentMetadata$DefaultLocalConfigurationMetadata.getExcludes(DefaultLocalComponentMetadata.java:460)
09:25:07     at org.gradle.internal.component.model.DefaultSelectedByVariantMatchingConfigurationMetadata.getExcludes(DefaultSelectedByVariantMatchingConfigurationMetadata.java:73)
09:25:07     at org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.builder.NodeState.computeNodeExclusions(NodeState.java:613)
09:25:07     at org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.builder.NodeState.computeModuleResolutionFilter(NodeState.java:603)
09:25:07     at org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.builder.NodeState.visitOutgoingDependencies(NodeState.java:250)
09:25:07     at org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.builder.DependencyGraphBuilder.traverseGraph(DependencyGraphBuilder.java:180)
09:25:07     at org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.builder.DependencyGraphBuilder.resolve(DependencyGraphBuilder.java:142)
09:25:07     at org.gradle.api.internal.artifacts.ivyservice.resolveengine.DefaultArtifactDependencyResolver.resolve(DefaultArtifactDependencyResolver.java:123)
09:25:07     at org.gradle.api.internal.artifacts.ivyservice.DefaultConfigurationResolver.resolveGraph(DefaultConfigurationResolver.java:175)
09:25:07     at org.gradle.api.internal.artifacts.ivyservice.ShortCircuitEmptyConfigurationResolver.resolveGraph(ShortCircuitEmptyConfigurationResolver.java:86)
09:25:07     at org.gradle.api.internal.artifacts.ivyservice.ErrorHandlingConfigurationResolver.resolveGraph(ErrorHandlingConfigurationResolver.java:74)

This is the branch from which the exception is thrown:

gradle/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/configurations/DefaultConfiguration.java

Lines 1131 to 1138 in bd168bb

	} else if (observedState == GRAPH_RESOLVED || observedState == ARTIFACTS_RESOLVED) {
	// The configuration has been used in a resolution, and it is an error for build logic to change any dependencies,
	// exclude rules or parent configurations (values that will affect the resolved graph).
	if (type != MutationType.STRATEGY) {
	String extraMessage = insideBeforeResolve ? " Use 'defaultDependencies' instead of 'beforeResolve' to specify default dependencies for a configuration." : "";
	throw new InvalidUserDataException(String.format("Cannot change %s of %s after it has been included in dependency resolution.%s", type, getDisplayName(), extraMessage));
	}
	}

It seems like the affected configuration has already been set to an observedState that disallows mutation (it is the parent of other configurations that are being resolved, as indicated by the recursive runDependencyActions frames in the stacktrace above) before this dependency action has been executed.

I am not familiar with the implementation details of Gradle's dependency resolution, but this sounds like it contradicts the Javadoc of withDependencies. Perhaps there are missing calls to runDependencyActions somewhere?

Your environment

This is on 5.6.3.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. If you're interested in how we try to keep the backlog in a healthy state, please read our blog post on how we refine our backlog. If you feel this is something you could contribute, please have a look at our Contributor Guide. Thank you for your contribution.

[stale]

This issue has been automatically closed due to inactivity. If you can reproduce this on a recent version of Gradle or if you have a good use case for this feature, please feel free to reopen the issue with steps to reproduce, a quick explanation of your use case or a high-quality pull request.

[lacasseio]

I'm having similar flakiness, it seems there is an unlinked Set somewhere in Gradle implementation causing dependencies to lock too early. I don't agree with the stale bot here. It closed a valid state issue. I agree it may be a deep issue that is rare but it's an issue nonetheless.

[lacasseio]

A bit more context that arrives at the same error. In the Nokee plugin, we need, before Gradle performs any kind of resolution, to perform last-minute changes to the Configuration. This is part of our code managing additional lazy configuration to support lots of variants inside the same project. We used the internal API beforeLocking but even that API sometimes gets called after the configuration has been resolved. The solution was to mix beforeLocking with defaultDependencies which both actions do the same thing. Whatever action gets called first gets to execute the logic.