Fix nasty concurrency issue in AbstractSession (#1479)

* Register the session explicitely
* Fix thread interrupted
This commit is contained in:
Guillaume Nodet 2024-04-23 14:55:57 +02:00 committed by GitHub
parent aae74dfbee
commit 52c5659b25
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 72 additions and 46 deletions

View File

@ -94,7 +94,6 @@ public abstract class AbstractSession implements InternalSession {
this.repositorySystem = repositorySystem;
this.repositories = getRepositories(repositories, resolverRepositories);
this.lookup = lookup;
this.session.getData().set(InternalSession.class, this);
}
@Override
@ -155,13 +154,13 @@ public abstract class AbstractSession implements InternalSession {
return dependencies == null ? null : map(dependencies, d -> toDependency(d, managed));
}
protected List<RemoteRepository> getRepositories(
static List<RemoteRepository> getRepositories(
List<RemoteRepository> repositories,
List<org.eclipse.aether.repository.RemoteRepository> resolverRepositories) {
if (repositories != null) {
return repositories;
} else if (resolverRepositories != null) {
return map(resolverRepositories, this::getRemoteRepository);
return map(resolverRepositories, DefaultRemoteRepository::new);
} else {
throw new IllegalArgumentException("no remote repositories provided");
}

View File

@ -45,6 +45,12 @@ public interface InternalSession extends Session {
return cast(InternalSession.class, session.getData().get(InternalSession.class), "session");
}
static void associate(org.eclipse.aether.RepositorySystemSession rsession, Session session) {
if (!rsession.getData().set(InternalSession.class, null, from(session))) {
throw new IllegalStateException("A maven session is already associated with the repository session");
}
}
RemoteRepository getRemoteRepository(org.eclipse.aether.repository.RemoteRepository repository);
Node getNode(org.eclipse.aether.graph.DependencyNode node);

View File

@ -414,6 +414,7 @@ public class ApiRunner {
.map(repositoryFactory::createRemote)
.toList();
InternalSession s = (InternalSession) session.withRemoteRepositories(repositories);
InternalSession.associate(rsession, s);
return s;
// List<RemoteRepository> repositories = repositoryFactory.createRemote();

View File

@ -35,6 +35,7 @@ import org.apache.maven.execution.DefaultMavenExecutionResult;
import org.apache.maven.execution.MavenExecutionRequest;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.internal.impl.DefaultSession;
import org.apache.maven.internal.impl.InternalSession;
import org.apache.maven.model.Build;
import org.apache.maven.model.Dependency;
import org.apache.maven.model.Exclusion;
@ -128,6 +129,7 @@ public abstract class AbstractCoreMavenComponentTestCase {
getContainer(), configuration.getRepositorySession(), request, new DefaultMavenExecutionResult());
DefaultSession iSession =
new DefaultSession(session, mock(org.eclipse.aether.RepositorySystem.class), null, null, null, null);
InternalSession.associate(session.getRepositorySession(), iSession);
session.setSession(iSession);
List<MavenProject> projects = new ArrayList<>();

View File

@ -33,6 +33,7 @@ import org.apache.maven.execution.DefaultMavenExecutionResult;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.internal.impl.DefaultLookup;
import org.apache.maven.internal.impl.DefaultSession;
import org.apache.maven.internal.impl.InternalSession;
import org.apache.maven.model.building.ModelBuildingException;
import org.apache.maven.model.building.ModelProblem;
import org.apache.maven.repository.RepositorySystem;
@ -153,12 +154,13 @@ public abstract class AbstractMavenProjectTestCase {
DefaultMavenExecutionRequest mavenExecutionRequest = new DefaultMavenExecutionRequest();
MavenSession msession =
new MavenSession(getContainer(), session, mavenExecutionRequest, new DefaultMavenExecutionResult());
new DefaultSession(
DefaultSession iSession = new DefaultSession(
msession,
mock(org.eclipse.aether.RepositorySystem.class),
null,
null,
new DefaultLookup(container),
null);
InternalSession.associate(session, iSession);
}
}

View File

@ -122,6 +122,7 @@ class LegacyRepositorySystemTest {
new MavenSession(container, session, mavenExecutionRequest, new DefaultMavenExecutionResult());
legacySupport.setSession(mavenSession);
InternalSession iSession = new DefaultSession(mavenSession, null, null, null, null, null);
InternalSession.associate(session, iSession);
ArtifactResolutionResult result = repositorySystem.resolve(request);
resolutionErrorHandler.throwErrors(request, result);

View File

@ -73,6 +73,9 @@ public class DefaultSession extends AbstractSession implements InternalMavenSess
}
public MavenSession getMavenSession() {
if (mavenSession == null) {
throw new IllegalArgumentException("Found null mavenSession on session " + this);
}
return mavenSession;
}
@ -94,19 +97,19 @@ public class DefaultSession extends AbstractSession implements InternalMavenSess
@Nonnull
@Override
public Settings getSettings() {
return mavenSession.getSettings().getDelegate();
return getMavenSession().getSettings().getDelegate();
}
@Nonnull
@Override
public Map<String, String> getUserProperties() {
return Collections.unmodifiableMap(new PropertiesAsMap(mavenSession.getUserProperties()));
return Collections.unmodifiableMap(new PropertiesAsMap(getMavenSession().getUserProperties()));
}
@Nonnull
@Override
public Map<String, String> getSystemProperties() {
return Collections.unmodifiableMap(new PropertiesAsMap(mavenSession.getSystemProperties()));
return Collections.unmodifiableMap(new PropertiesAsMap(getMavenSession().getSystemProperties()));
}
@Nonnull
@ -128,29 +131,29 @@ public class DefaultSession extends AbstractSession implements InternalMavenSess
@Override
public int getDegreeOfConcurrency() {
return mavenSession.getRequest().getDegreeOfConcurrency();
return getMavenSession().getRequest().getDegreeOfConcurrency();
}
@Nonnull
@Override
public Instant getStartTime() {
return mavenSession.getRequest().getStartTime().toInstant();
return getMavenSession().getRequest().getStartTime().toInstant();
}
@Override
public Path getRootDirectory() {
return mavenSession.getRequest().getRootDirectory();
return getMavenSession().getRequest().getRootDirectory();
}
@Override
public Path getTopDirectory() {
return mavenSession.getRequest().getTopDirectory();
return getMavenSession().getRequest().getTopDirectory();
}
@Nonnull
@Override
public List<Project> getProjects() {
return getProjects(mavenSession.getProjects());
return getProjects(getMavenSession().getProjects());
}
@Nonnull
@ -161,14 +164,14 @@ public class DefaultSession extends AbstractSession implements InternalMavenSess
MojoExecution mojoExecution = lookup.lookup(MojoExecution.class);
MojoDescriptor mojoDescriptor = mojoExecution.getMojoDescriptor();
PluginDescriptor pluginDescriptor = mojoDescriptor.getPluginDescriptor();
return mavenSession.getPluginContext(pluginDescriptor, ((DefaultProject) project).getProject());
return getMavenSession().getPluginContext(pluginDescriptor, ((DefaultProject) project).getProject());
} catch (LookupException e) {
throw new MavenException("The PluginContext is only available during a mojo execution", e);
}
}
protected Session newSession(RepositorySystemSession repoSession, List<RemoteRepository> repositories) {
final MavenSession ms = nonNull(mavenSession);
final MavenSession ms = nonNull(getMavenSession());
final MavenSession mss;
if (repoSession != ms.getRepositorySession()) {
mss = new MavenSession(repoSession, ms.getRequest(), ms.getResult());

View File

@ -50,7 +50,9 @@ public class DefaultSessionFactory {
}
public InternalSession newSession(MavenSession mavenSession) {
return new DefaultSession(
InternalSession session = new DefaultSession(
mavenSession, repositorySystem, null, mavenRepositorySystem, lookup, runtimeInformation);
InternalSession.associate(mavenSession.getRepositorySession(), session);
return session;
}
}

View File

@ -28,6 +28,7 @@ import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ForkJoinPool;
import java.util.concurrent.ForkJoinTask;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.maven.api.model.Dependency;
@ -85,7 +86,7 @@ public class ProjectModelResolver implements ModelResolver {
private final ProjectBuildingRequest.RepositoryMerging repositoryMerging;
private final Map<String, ForkJoinTask<Result>> parentCache;
private final Map<String, Future<Result>> parentCache;
public ProjectModelResolver(
RepositorySystemSession session,
@ -189,7 +190,9 @@ public class ProjectModelResolver implements ModelResolver {
throws UnresolvableModelException {
Result result;
try {
ForkJoinTask<Result> future = parentCache.computeIfAbsent(parent.getId(), id -> new ForkJoinTask<>() {
Future<Result> future = parentCache.computeIfAbsent(parent.getId(), id -> {
ForkJoinPool pool = new ForkJoinPool(MAX_CAP);
ForkJoinTask<Result> task = new ForkJoinTask<>() {
Result result;
@Override
@ -210,17 +213,16 @@ public class ProjectModelResolver implements ModelResolver {
result = new Result(source, modified.get(), null);
} catch (Exception e) {
result = new Result(null, null, e);
} finally {
pool.shutdown();
}
return true;
}
};
pool.submit(task);
return task;
});
ForkJoinPool pool = new ForkJoinPool(MAX_CAP);
try {
pool.execute(future);
result = future.get();
} finally {
pool.shutdownNow();
}
} catch (Exception e) {
throw new UnresolvableModelException(e, parent.getGroupId(), parent.getArtifactId(), parent.getVersion());
}

View File

@ -23,6 +23,7 @@ import org.apache.maven.execution.DefaultMavenExecutionRequest;
import org.apache.maven.execution.DefaultMavenExecutionResult;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.internal.impl.DefaultSession;
import org.apache.maven.internal.impl.InternalSession;
import org.eclipse.aether.DefaultRepositorySystemSession;
public class MavenTestHelper {
@ -31,6 +32,7 @@ public class MavenTestHelper {
DefaultMavenExecutionRequest request = new DefaultMavenExecutionRequest();
MavenSession mavenSession = new MavenSession(repoSession, request, new DefaultMavenExecutionResult());
DefaultSession session = new DefaultSession(mavenSession, null, null, repositorySystem, null, null);
InternalSession.associate(repoSession, session);
return repoSession;
}
}

View File

@ -123,6 +123,7 @@ class TestApi {
new RemoteRepository.Builder("mirror", "default", "file:target/test-classes/repo").build());
this.session = session.withLocalRepository(localRepository)
.withRemoteRepositories(Collections.singletonList(remoteRepository));
InternalSession.associate(rss, this.session);
sessionScope.enter();
sessionScope.seed(InternalMavenSession.class, InternalMavenSession.from(this.session));
}

View File

@ -26,6 +26,7 @@ import org.apache.maven.execution.DefaultMavenExecutionRequest;
import org.apache.maven.execution.DefaultMavenExecutionResult;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.internal.impl.DefaultSession;
import org.apache.maven.internal.impl.InternalSession;
import org.codehaus.plexus.PlexusContainer;
import org.codehaus.plexus.testing.PlexusTest;
import org.eclipse.aether.DefaultRepositorySystemSession;
@ -71,6 +72,7 @@ public abstract class AbstractRepositoryTestCase {
DefaultMavenExecutionRequest request = new DefaultMavenExecutionRequest();
MavenSession mavenSession = new MavenSession(rsession, request, new DefaultMavenExecutionResult());
DefaultSession session = new DefaultSession(mavenSession, null, null, null, null, null);
InternalSession.associate(rsession, session);
return rsession;
}

View File

@ -70,6 +70,7 @@ public class ModelBuilderTest {
MavenSession msession = new MavenSession(rsession, mavenRequest, new DefaultMavenExecutionResult());
InternalSession session =
new DefaultSession(msession, repositorySystem, null, mavenRepositorySystem, null, null);
InternalSession.associate(rsession, session);
List<ProjectBuildingResult> results = projectBuilder.build(
Collections.singletonList(new File("src/test/resources/projects/tree/pom.xml")), true, request);

View File

@ -53,6 +53,7 @@ import org.apache.maven.internal.impl.DefaultRepositoryFactory;
import org.apache.maven.internal.impl.DefaultSession;
import org.apache.maven.internal.impl.DefaultVersionParser;
import org.apache.maven.internal.impl.DefaultVersionRangeResolver;
import org.apache.maven.internal.impl.InternalSession;
import org.apache.maven.plugin.PluginResolutionException;
import org.apache.maven.plugin.internal.DefaultPluginDependenciesResolver;
import org.apache.maven.resolver.MavenChainedWorkspaceReader;
@ -130,7 +131,8 @@ public class BootstrapCoreExtensionManager {
.setWorkspaceReader(new MavenChainedWorkspaceReader(request.getWorkspaceReader(), ideWorkspaceReader))
.build()) {
MavenSession mSession = new MavenSession(repoSession, request, new DefaultMavenExecutionResult());
new SimpleSession(mSession, repoSystem, null);
InternalSession iSession = new SimpleSession(mSession, repoSystem, null);
InternalSession.associate(repoSession, iSession);
List<RemoteRepository> repositories = RepositoryUtils.toRepos(request.getPluginArtifactRepositories());
Interpolator interpolator = createInterpolator(request);