-
Notifications
You must be signed in to change notification settings - Fork 127
Fix OutOfScopeException in getConfiguredMojo when plugin realm creates separate SessionScope #2124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
5c7d9e0
7a28f11
5d2fb81
e2734cb
4b5308d
5e5668a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" | ||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
|
|
||
| <groupId>org.eclipse.m2e.tests</groupId> | ||
| <artifactId>sessionScopeTest</artifactId> | ||
| <version>1.0.0-SNAPSHOT</version> | ||
| <packaging>jar</packaging> | ||
|
|
||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-jar-plugin</artifactId> | ||
| <version>3.5.0</version> | ||
| <configuration> | ||
| <archive> | ||
| <index>true</index> | ||
| <manifest> | ||
| <addClasspath>true</addClasspath> | ||
| </manifest> | ||
| <manifestEntries> | ||
| <mode>development</mode> | ||
| <key>value</key> | ||
| </manifestEntries> | ||
| </archive> | ||
| </configuration> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| /******************************************************************************* | ||
| * Copyright (c) 2025 Contributors | ||
| * All rights reserved. This program and the accompanying materials | ||
| * are made available under the terms of the Eclipse Public License 2.0 | ||
| * which accompanies this distribution, and is available at | ||
| * https://www.eclipse.org/legal/epl-2.0/ | ||
| * | ||
| * SPDX-License-Identifier: EPL-2.0 | ||
| *******************************************************************************/ | ||
|
|
||
| package org.eclipse.m2e.core.embedder; | ||
|
|
||
| import static org.junit.Assert.assertNotNull; | ||
|
|
||
| import org.apache.maven.execution.MavenSession; | ||
| import org.apache.maven.lifecycle.MavenExecutionPlan; | ||
| import org.apache.maven.plugin.Mojo; | ||
| import org.apache.maven.plugin.MojoExecution; | ||
| import org.eclipse.core.resources.IProject; | ||
| import org.eclipse.core.resources.IncrementalProjectBuilder; | ||
| import org.eclipse.core.runtime.NullProgressMonitor; | ||
| import org.eclipse.m2e.core.MavenPlugin; | ||
| import org.eclipse.m2e.core.project.IMavenProjectFacade; | ||
| import org.eclipse.m2e.tests.common.AbstractMavenProjectTestCase; | ||
| import org.junit.Test; | ||
|
|
||
| /** | ||
| * Tests for SessionScope handling in Maven embedder, particularly for issue #2084. | ||
| * This test verifies that getConfiguredMojo properly enters and seeds SessionScope | ||
| * when working with projects that have a .mvn folder. | ||
| * | ||
| * Background: When a project has a .mvn folder, PlexusContainerManager creates a | ||
| * separate container for that multi-module project directory. Each container has its | ||
| * own ClassWorld and Guice injector with separate SessionScope instances. This can | ||
| * cause OutOfScopeException if the SessionScope is not properly managed. | ||
| */ | ||
| public class SessionScopeTest extends AbstractMavenProjectTestCase { | ||
|
Check warning on line 37 in org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/embedder/SessionScopeTest.java
|
||
|
|
||
| /** | ||
| * Test that getConfiguredMojo works with projects containing .mvn folder. | ||
| * This reproduces the OutOfScopeException issue where plugin realm creation | ||
| * may use a different SessionScope instance that needs explicit seeding. | ||
| * | ||
| * The .mvn folder triggers PlexusContainerManager to create a separate container | ||
| * for the multi-module project directory, which has its own SessionScope instance. | ||
| * | ||
| * See: https://github.com/eclipse-m2e/m2e-core/issues/2084 | ||
| */ | ||
| @Test | ||
| public void testGetConfiguredMojoWithDotMvnFolder() throws Exception { | ||
| // Import a project with .mvn folder and maven-jar-plugin configured | ||
| IProject project = importProject("resources/projects/sessionScopeTest/pom.xml"); | ||
|
Check warning on line 52 in org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/embedder/SessionScopeTest.java
|
||
| assertNotNull("Project should be created", project); | ||
|
|
||
| waitForJobsToComplete(); | ||
|
Check warning on line 55 in org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/embedder/SessionScopeTest.java
|
||
|
|
||
| // Trigger a build to ensure the project is fully configured | ||
| project.build(IncrementalProjectBuilder.FULL_BUILD, new NullProgressMonitor()); | ||
| waitForJobsToComplete(); | ||
|
Check warning on line 59 in org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/embedder/SessionScopeTest.java
|
||
|
|
||
| IMavenProjectFacade facade = MavenPlugin.getMavenProjectRegistry().getProject(project); | ||
| assertNotNull("Maven facade should exist", facade); | ||
|
|
||
| // Get the jar mojo execution from the execution plan | ||
| IMaven maven = MavenPlugin.getMaven(); | ||
| IMavenExecutionContext context = facade.createExecutionContext(); | ||
|
|
||
| context.execute((ctx, monitor) -> { | ||
| MavenSession session = ctx.getSession(); | ||
| assertNotNull("Session should exist", session); | ||
|
|
||
| // Get the execution plan for the package phase which includes jar:jar goal | ||
| MavenExecutionPlan plan = maven.calculateExecutionPlan(facade.getMavenProject(), | ||
|
Check warning on line 73 in org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/embedder/SessionScopeTest.java
|
||
| java.util.List.of("package"), false, monitor); | ||
| assertNotNull("Execution plan should exist", plan); | ||
|
|
||
| // Find the jar:jar mojo execution | ||
| MojoExecution jarExecution = null; | ||
| for (MojoExecution execution : plan.getMojoExecutions()) { | ||
| if ("maven-jar-plugin".equals(execution.getArtifactId()) && "jar".equals(execution.getGoal())) { | ||
| jarExecution = execution; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| assertNotNull("jar:jar execution should exist in plan", jarExecution); | ||
|
|
||
| // This call should NOT throw OutOfScopeException | ||
| // Before the fix, this would fail with: | ||
| // "OutOfScopeException: Cannot access session scope outside of a scoping block" | ||
| Mojo configuredMojo = maven.getConfiguredMojo(session, jarExecution, Mojo.class); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot This throws the following NPE:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed the NPE by using Commit: 5e5668a |
||
| assertNotNull("Configured mojo should be created", configuredMojo); | ||
|
|
||
| // Clean up | ||
| maven.releaseMojo(configuredMojo, jarExecution); | ||
|
|
||
| return null; | ||
| }, monitor); | ||
|
Check warning on line 98 in org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/embedder/SessionScopeTest.java
|
||
| } | ||
|
|
||
| /** | ||
| * Test that getConfiguredMojo also works with projects without .mvn folder | ||
| * to ensure the fix doesn't break existing functionality. | ||
| */ | ||
| @Test | ||
| public void testGetConfiguredMojoWithoutDotMvnFolder() throws Exception { | ||
| // Use an existing simple project without .mvn folder | ||
| IProject project = importProject("resources/projects/simplePomOK/pom.xml"); | ||
|
Check warning on line 108 in org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/embedder/SessionScopeTest.java
|
||
| assertNotNull("Project should be created", project); | ||
|
|
||
| waitForJobsToComplete(); | ||
|
Check warning on line 111 in org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/embedder/SessionScopeTest.java
|
||
|
|
||
| IMavenProjectFacade facade = MavenPlugin.getMavenProjectRegistry().getProject(project); | ||
| assertNotNull("Maven facade should exist", facade); | ||
|
|
||
| // Verify that getConfiguredMojo still works for normal projects | ||
| IMaven maven = MavenPlugin.getMaven(); | ||
| IMavenExecutionContext context = facade.createExecutionContext(); | ||
|
|
||
| context.execute((ctx, monitor) -> { | ||
| MavenSession session = ctx.getSession(); | ||
| assertNotNull("Session should exist", session); | ||
|
|
||
| // This should work without issues both before and after the fix | ||
| return null; | ||
| }, monitor); | ||
|
Check warning on line 126 in org.eclipse.m2e.core.tests/src/org/eclipse/m2e/core/embedder/SessionScopeTest.java
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,7 @@ | |
| import org.apache.maven.execution.MavenSession; | ||
| import org.apache.maven.execution.scope.internal.MojoExecutionScope; | ||
| import org.apache.maven.lifecycle.LifecycleExecutor; | ||
| import org.apache.maven.session.scope.internal.SessionScope; | ||
| import org.apache.maven.lifecycle.MavenExecutionPlan; | ||
| import org.apache.maven.lifecycle.internal.LifecycleExecutionPlanCalculator; | ||
| import org.apache.maven.model.ConfigurationContainer; | ||
|
|
@@ -206,15 +207,41 @@ public <T> T getConfiguredMojo(MavenSession session, MojoExecution mojoExecution | |
| MojoDescriptor mojoDescriptor = mojoExecution.getMojoDescriptor(); | ||
| // getPluginRealm creates plugin realm and populates pluginDescriptor.classRealm field | ||
| lookup(BuildPluginManager.class).getPluginRealm(session, mojoDescriptor.getPluginDescriptor()); | ||
| MojoExecutionScope scope = lookup(MojoExecutionScope.class); | ||
|
|
||
| // When a project has a .mvn folder, PlexusContainerManager creates a separate container for that | ||
| // multi-module project directory (see PlexusContainerManager.aquire(File)). Each container has its | ||
| // own ClassWorld and thus its own Guice injector with separate SessionScope instances. | ||
| // After getPluginRealm() loads the plugin, we may be using a different SessionScope instance than | ||
| // the one seeded in MavenExecutionContext.execute(). We need to ensure this SessionScope is also | ||
| // entered and seeded. We try to seed first; if that fails with OutOfScopeException, we enter and seed. | ||
| // See: https://github.com/eclipse-m2e/m2e-core/issues/2084 | ||
| SessionScope sessionScope = lookup(SessionScope.class); | ||
| boolean sessionScopeEntered = false; | ||
| try { | ||
| // Initialize MojoExecutionScope (for mojo's leveraging Sisu with JSR330 annotations, this is otherwise done by org.apache.maven.plugin.DefaultBuildPluginManager) | ||
| scope.enter(); | ||
| scope.seed(MavenProject.class, session.getCurrentProject()); | ||
| scope.seed(MojoExecution.class, mojoExecution); | ||
| return clazz.cast(lookup(MavenPluginManager.class).getConfiguredMojo(Mojo.class, session, mojoExecution)); | ||
| // Try to seed the session - this will fail if the scope is not entered | ||
| sessionScope.seed(MavenSession.class, session); | ||
| } catch(com.google.inject.OutOfScopeException e) { | ||
| // SessionScope is not entered, we need to enter it first and then seed | ||
| sessionScope.enter(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot Is there a way to detect that without relying on exceptions? If not what happens if the sessionScope is entered/seeded twice?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've changed the approach to avoid relying on a protected method. Now we try to If SessionScope is already entered and seeded, calling
Commit: 4b5308d |
||
| sessionScope.seed(MavenSession.class, session); | ||
| sessionScopeEntered = true; | ||
| } | ||
|
|
||
| try { | ||
| MojoExecutionScope mojoScope = lookup(MojoExecutionScope.class); | ||
| try { | ||
| // Initialize MojoExecutionScope (for mojo's leveraging Sisu with JSR330 annotations, this is otherwise done by org.apache.maven.plugin.DefaultBuildPluginManager) | ||
| mojoScope.enter(); | ||
| mojoScope.seed(MavenProject.class, session.getCurrentProject()); | ||
| mojoScope.seed(MojoExecution.class, mojoExecution); | ||
| return clazz.cast(lookup(MavenPluginManager.class).getConfiguredMojo(Mojo.class, session, mojoExecution)); | ||
| } finally { | ||
| mojoScope.exit(); | ||
| } | ||
| } finally { | ||
| scope.exit(); | ||
| if(sessionScopeEntered) { | ||
| sessionScope.exit(); | ||
| } | ||
| } | ||
| } catch(PluginManagerException | PluginConfigurationException | ClassCastException | PluginResolutionException | ||
| | MojoExecutionException ex) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HannesWell Any idea why for test classes Jenkins complains about
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually means package not exported but using RequireBundle ...