Skip to content

Commit

Permalink
Stop accepting selection changes
Browse files Browse the repository at this point in the history 
Prior to this change, in some corner cases, dependency graph building
would never terminate, with selection of some modules alternating
repeatedly.
With these changes, we now ignore selection changes after a high
threshold. This prevents unstable graph to loop in resolution for ever.

Fixes #13450
  • Loading branch information
@ljacomet
ljacomet committed Jul 24, 2020
1 parent d2ba516 commit 5d7efbd
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 5 deletions.
2 changes: 1 addition & 1 deletion ...api/internal/artifacts/ivyservice/resolveengine/graph/builder/DependencyGraphBuilder.java
View file
Expand Up @@ -139,7 +139,7 @@ public void resolve(final ResolveContext resolveContext, final DependencyGraphVi
int graphSize = estimateSize(resolveContext);
ResolutionStrategyInternal resolutionStrategy = resolveContext.getResolutionStrategy();

final ResolveState resolveState = new ResolveState(idGenerator, rootModule, resolveContext.getName(), idResolver, metaDataResolver, edgeFilter, attributesSchema, moduleExclusions, componentSelectorConverter, attributesFactory, dependencySubstitutionApplicator, versionSelectorScheme, versionComparator, versionParser, moduleConflictHandler.getResolver(), graphSize);
final ResolveState resolveState = new ResolveState(idGenerator, rootModule, resolveContext.getName(), idResolver, metaDataResolver, edgeFilter, attributesSchema, moduleExclusions, componentSelectorConverter, attributesFactory, dependencySubstitutionApplicator, versionSelectorScheme, versionComparator, versionParser, moduleConflictHandler.getResolver(), graphSize, resolveContext.getResolutionStrategy().getConflictResolution());

Map<ModuleVersionIdentifier, ComponentIdentifier> componentIdentifierCache = Maps.newHashMapWithExpectedSize(graphSize / 2);
traverseGraph(resolveState, componentIdentifierCache);
Expand Down
40 changes: 38 additions & 2 deletions ...dle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ModuleResolveState.java
View file
Expand Up @@ -23,7 +23,9 @@
import org.gradle.api.artifacts.component.ComponentIdentifier;
import org.gradle.api.artifacts.component.ComponentSelector;
import org.gradle.api.artifacts.component.ModuleComponentIdentifier;
import org.gradle.api.artifacts.component.ProjectComponentIdentifier;
import org.gradle.api.attributes.AttributeContainer;
import org.gradle.api.internal.artifacts.configurations.ConflictResolution;
import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.Version;
import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.VersionParser;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.conflicts.CandidateModule;
Expand All @@ -36,6 +38,8 @@
import org.gradle.internal.component.model.ForcingDependencyMetadata;
import org.gradle.internal.id.IdGenerator;
import org.gradle.internal.resolve.resolver.ComponentMetaDataResolver;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nullable;
import java.util.ArrayList;
Expand All @@ -52,12 +56,16 @@
* Resolution state for a given module.
*/
class ModuleResolveState implements CandidateModule {
private static final Logger LOGGER = LoggerFactory.getLogger(ModuleResolveState.class);
private static final int MAX_SELECTION_CHANGE = 1000;

private final ComponentMetaDataResolver metaDataResolver;
private final IdGenerator<Long> idGenerator;
private final ModuleIdentifier id;
private final List<EdgeState> unattachedDependencies = new LinkedList<>();
private final Map<ModuleVersionIdentifier, ComponentState> versions = new LinkedHashMap<>();
private final ModuleSelectors<SelectorState> selectors;
private final ConflictResolution conflictResolution;
private final ImmutableAttributesFactory attributesFactory;
private final Comparator<Version> versionComparator;
private final VersionParser versionParser;
Expand All @@ -74,6 +82,7 @@ class ModuleResolveState implements CandidateModule {
private Set<VirtualPlatformState> platformOwners;
private boolean replaced = false;
private boolean changingSelection;
private int selectionChangedCounter;

ModuleResolveState(IdGenerator<Long> idGenerator,
ModuleIdentifier id,
Expand All @@ -82,7 +91,9 @@ class ModuleResolveState implements CandidateModule {
Comparator<Version> versionComparator,
VersionParser versionParser,
SelectorStateResolver<ComponentState> selectorStateResolver,
ResolveOptimizations resolveOptimizations, boolean rootModule) {
ResolveOptimizations resolveOptimizations,
boolean rootModule,
ConflictResolution conflictResolution) {
this.idGenerator = idGenerator;
this.id = id;
this.metaDataResolver = metaDataResolver;
Expand All @@ -93,7 +104,8 @@ class ModuleResolveState implements CandidateModule {
this.rootModule = rootModule;
this.pendingDependencies = new PendingDependencies(id);
this.selectorStateResolver = selectorStateResolver;
selectors = new ModuleSelectors<SelectorState>(versionComparator);
this.selectors = new ModuleSelectors<>(versionComparator);
this.conflictResolution = conflictResolution;
}

void setSelectorStateResolver(SelectorStateResolver<ComponentState> selectorStateResolver) {
Expand Down Expand Up @@ -411,10 +423,34 @@ public void maybeUpdateSelection() {
if (selected == null) {
select(newSelected);
} else if (newSelected != selected) {
if (++selectionChangedCounter > MAX_SELECTION_CHANGE) {
// Let's ignore modules that are changing selection way too much, by keeping the highest version
if (maybeSkipSelectionChange(newSelected)) {
return;
}
}
changeSelection(newSelected);
}
}

private boolean maybeSkipSelectionChange(ComponentState newSelected) {
if (selectionChangedCounter == MAX_SELECTION_CHANGE + 1) {
LOGGER.warn("The dependency resolution engine wasn't able to find a version of module {} which satisfied all requirements because the graph wasn't stable enough. " +
"The highest version was selected in order to stabilize selection.\n" +
"Features available in a stable graph like version alignment are not guaranteed in this case.", id);
}
boolean newSelectedIsProject = false;
if (conflictResolution == ConflictResolution.preferProjectModules) {
if (newSelected.getComponentId() instanceof ProjectComponentIdentifier) {
// Keep the project selected
newSelectedIsProject = true;
}
}
Version newVersion = versionParser.transform(newSelected.getVersion());
Version currentVersion = versionParser.transform(selected.getVersion());
return !newSelectedIsProject && versionComparator.compare(newVersion, currentVersion) <= 0;
}

void maybeCreateVirtualMetadata(ResolveState resolveState) {
for (ComponentState componentState : versions.values()) {
if (componentState.getMetadata() == null) {
Expand Down
8 changes: 6 additions & 2 deletions ...rg/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ResolveState.java
View file
Expand Up @@ -26,6 +26,7 @@
import org.gradle.api.internal.artifacts.ComponentSelectorConverter;
import org.gradle.api.internal.artifacts.ResolvedConfigurationIdentifier;
import org.gradle.api.internal.artifacts.ResolvedVersionConstraint;
import org.gradle.api.internal.artifacts.configurations.ConflictResolution;
import org.gradle.api.internal.artifacts.dependencies.DefaultResolvedVersionConstraint;
import org.gradle.api.internal.artifacts.ivyservice.dependencysubstitution.DependencySubstitutionApplicator;
import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.Version;
Expand Down Expand Up @@ -70,6 +71,7 @@ class ResolveState implements ComponentStateFactory<ComponentState> {
private final DependencyToComponentIdResolver idResolver;
private final ComponentMetaDataResolver metaDataResolver;
private final Deque<NodeState> queue;
private final ConflictResolution conflictResolution;
private final AttributesSchemaInternal attributesSchema;
private final ModuleExclusions moduleExclusions;
private final DeselectVersionAction deselectVersionAction = new DeselectVersionAction(this);
Expand Down Expand Up @@ -100,7 +102,8 @@ public ResolveState(IdGenerator<Long> idGenerator,
Comparator<Version> versionComparator,
VersionParser versionParser,
ModuleConflictResolver<ComponentState> conflictResolver,
int graphSize) {
int graphSize,
ConflictResolution conflictResolution) {
this.idGenerator = idGenerator;
this.idResolver = idResolver;
this.metaDataResolver = metaDataResolver;
Expand All @@ -117,6 +120,7 @@ public ResolveState(IdGenerator<Long> idGenerator,
this.nodes = new LinkedHashMap<>(3 * graphSize / 2);
this.selectors = new LinkedHashMap<>(5 * graphSize / 2);
this.queue = new ArrayDeque<>(graphSize);
this.conflictResolution = conflictResolution;
this.resolveOptimizations = new ResolveOptimizations();
this.attributeDesugaring = new AttributeDesugaring(attributesFactory);
// Create root module
Expand Down Expand Up @@ -149,7 +153,7 @@ public ModuleResolveState getModule(ModuleIdentifier id) {
}

private ModuleResolveState getModule(ModuleIdentifier id, boolean rootModule) {
return modules.computeIfAbsent(id, mid -> new ModuleResolveState(idGenerator, id, metaDataResolver, attributesFactory, versionComparator, versionParser, selectorStateResolver, resolveOptimizations, rootModule));
return modules.computeIfAbsent(id, mid -> new ModuleResolveState(idGenerator, id, metaDataResolver, attributesFactory, versionComparator, versionParser, selectorStateResolver, resolveOptimizations, rootModule, conflictResolution));
}

@Override
Expand Down

0 comments on commit 5d7efbd

Please sign in to comment.