Fix OutOfScopeException in getConfiguredMojo when plugin realm creates separate SessionScope #2124
Conversation
|
FYI @kwin I asked the AI to analyze why your fix is not effective in all cases, maybe this can be used as an inspiration. |
…nfiguredMojo Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
|
@copilot Can you create a testcase for this in https://github.com/eclipse-m2e/m2e-core/tree/main/org.eclipse.m2e.core.tests ? At best it would fail before this change and succeed after this change so we make sure this will not get any regression in the future. |
Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
I've created a test case in The test includes:
Test project is in Commit: 5d2fb81 |
|
@copilot Can you only seed the SessionScope in case this is not done yet? Also elaborate why with a |
…n for .mvn behavior Co-authored-by: kwin <185025+kwin@users.noreply.github.com>
I've updated the code to only seed SessionScope if not already active, using Why .mvn folders cause different Guice injector: When a project has a
The code now checks if SessionScope is already active before entering/seeding it, and only exits if we entered it. Commit: e2734cb |
| scope.seed(MojoExecution.class, mojoExecution); | ||
| return clazz.cast(lookup(MavenPluginManager.class).getConfiguredMojo(Mojo.class, session, mojoExecution)); | ||
| // Check if SessionScope is already active by attempting to get its state | ||
| sessionScope.getScopeState(); |
There was a problem hiding this comment.
@copilot Isn't this method protected in Maven 3.9: https://github.com/apache/maven/blob/56df59baea62dcaf6c5db93392a58e871f968ef6/maven-core/src/main/java/org/apache/maven/session/scope/internal/SessionScope.java#L68?
| sessionScope.getScopeState(); | ||
| } catch(com.google.inject.OutOfScopeException e) { | ||
| // SessionScope is not active, we need to enter and seed it | ||
| sessionScope.enter(); |
There was a problem hiding this comment.
@copilot Is there a way to detect that without relying on exceptions? If not what happens if the sessionScope is entered/seeded twice?
There was a problem hiding this comment.
I've changed the approach to avoid relying on a protected method. Now we try to seed() directly (which is public), and catch OutOfScopeException if the scope is not entered.
If SessionScope is already entered and seeded, calling seed() again will succeed - it simply replaces the existing seed value with the same MavenSession object. This is safe and won't cause issues since:
- We're seeding with the same session object that should already be there
- We only exit the scope if we entered it (tracked by
sessionScopeEnteredflag) - This prevents issues with nested scope management
Commit: 4b5308d
…getScopeState() Co-authored-by: kwin <185025+kwin@users.noreply.github.com>
| // 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); |
There was a problem hiding this comment.
@copilot This throws the following NPE:
java.lang.NullPointerException: Cannot invoke "org.apache.maven.plugin.descriptor.MojoDescriptor.getPluginDescriptor()" because "mojoDescriptor" is null
at org.eclipse.m2e.core.internal.embedder.MavenImpl.getConfiguredMojo(MavenImpl.java:209)
at org.eclipse.m2e.core.embedder.SessionScopeTest.lambda$0(SessionScopeTest.java:91)
at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.executeBare(MavenExecutionContext.java:462)
at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.execute(MavenExecutionContext.java:342)
at org.eclipse.m2e.core.internal.embedder.MavenExecutionContext.execute(MavenExecutionContext.java:281)
at org.eclipse.m2e.core.embedder.SessionScopeTest.testGetConfiguredMojoWithDotMvnFolder(SessionScopeTest.java:68)
There was a problem hiding this comment.
Fixed the NPE by using facade.setupExecutionPlan() instead of maven.calculateExecutionPlan(). The issue was that calculateExecutionPlan() doesn't properly initialize the MojoDescriptor, while setupExecutionPlan() does. This matches how the production code in AbstractMavenArchiverConfigurator obtains the execution plan.
Commit: 5e5668a
…tionPlan Co-authored-by: kwin <185025+kwin@users.noreply.github.com>
| * 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 { |
There was a problem hiding this comment.
@HannesWell Any idea why for test classes Jenkins complains about
Discouraged access: The type 'AbstractMavenProjectTestCase' is not API (restriction on classpath entry '/home/jenkins/agent/workspace/m2e_PR-2124/org.eclipse.m2e.tests.common/target/classes')
There was a problem hiding this comment.
Usually means package not exported but using RequireBundle ...
|
@kwin ist usually a good idea to fetch the branch and squash the commits to keep the history clean from intermediate steps. |
Fix OutOfScopeException in getConfiguredMojo when plugin realm creates separate SessionScope
Changes
Improved SessionScope handling in
MavenImpl.getConfiguredMojo():Conditional seeding with public API: Check if SessionScope is already entered using
seed()instead of protectedgetScopeState()OutOfScopeExceptionis thrown, it means the scope is not entered, so we enter and then seedsessionScopeEnteredflag)What happens if SessionScope is already seeded:
seed()call will succeed (it replaces the existing seed)Detailed documentation: Added comprehensive comments explaining:
.mvnfolders trigger the issuePlexusContainerManager.aquire(File)creates separate containers for multi-module directoriesgetPluginRealm()is calledWhy .mvn Folders Cause This Issue
When a project has a
.mvnfolder:MavenProperties.computeMultiModuleProjectDirectory()identifies it as a multi-module rootPlexusContainerManager.aquire(File)creates a separate PlexusContainer for that directoryClassWorldinstance (to avoidDuplicateRealmExceptionfor extensions)ClassWorldhas its own Guice injector with separateSessionScopeinstancesgetPluginRealm()loads the plugin into this separate container, we're using a differentSessionScopethan the one seeded inMavenExecutionContext.execute()OutOfScopeExceptionTest Coverage
Added
SessionScopeTestinorg.eclipse.m2e.core.tests:calculateExecutionPlan()tofacade.setupExecutionPlan()which properly initializes the MojoDescriptortestGetConfiguredMojoWithDotMvnFolder(): Tests the fix with a project containing .mvn foldertestGetConfiguredMojoWithoutDotMvnFolder(): Regression test for projects without .mvn folderOriginal prompt
This section details on the original issue you should resolve
<issue_title>OutOfScopeException: Cannot access session scope outside of a scoping block</issue_title>
<issue_description>Hello,
I have (probably) the same issue as #2084 (I can't reopen it but feel free to close this one and reopen the other).
I decided to use the Snapshot repository ( https://download.eclipse.org/technology/m2e/snapshots/latest ) to get the fix but it still seems to fail, although the error message is a little bit different:
OutOfScopeException: Cannot access session scope outside of a scoping block.For information:
The trace: