Affected version
1.0.0
Bug description
EmbeddedMavenExecutor copies System.getProperties() once, in its constructor (EmbeddedMavenExecutor.java:128-129), together with System.out and System.err, and restores that same snapshot in the finally block of every execute() (:159-162). Any change a caller makes to system properties between two executions on one long-lived executor is silently reverted after the next run. maven-verifier's Embedded3xLauncher snapshotted per run.
The pattern that triggers it is the natural one for a test fixture: one ExecutorHelper per JVM, reused across hundreds of integration tests, some of which set a system property to drive the next build. Found while porting maven-surefire's IT fixture (apache/maven-surefire#3484); the fixture now creates one executor per Verifier instance to avoid it.
Proposal: take the snapshot at the start of each execute() rather than in the constructor. This is distinct from the thread-safety window in #16 item 8, which is about System.setProperties(null) during a run.
The same JVM-lifetime pattern is in apache/maven itself: its/core-it-support/maven-it-helper/.../Verifier.java holds static final EmbeddedMavenExecutor and ForkedMavenExecutor instances, so the core ITs on master and on the 3.x branches (apache/maven-integration-testing#445) are exposed as well; fixing the snapshot point in the executor covers all of them.
The obvious workaround, one EmbeddedMavenExecutor per execution closed afterwards, does not work at scale: on apache/maven-surefire#3484 (run 35594937350) the IT JVM runs out of heap after roughly a hundred builds (java.lang.OutOfMemoryError: Java heap space inside the embedded Maven), because close() does not release the Maven ClassWorld the constructor built. So callers are pushed back to a JVM-lifetime executor and inherit the snapshot behaviour; moving the snapshot into execute() is the fix that avoids both.
Affected version
1.0.0
Bug description
EmbeddedMavenExecutorcopiesSystem.getProperties()once, in its constructor (EmbeddedMavenExecutor.java:128-129), together withSystem.outandSystem.err, and restores that same snapshot in thefinallyblock of everyexecute()(:159-162). Any change a caller makes to system properties between two executions on one long-lived executor is silently reverted after the next run. maven-verifier'sEmbedded3xLaunchersnapshotted per run.The pattern that triggers it is the natural one for a test fixture: one
ExecutorHelperper JVM, reused across hundreds of integration tests, some of which set a system property to drive the next build. Found while portingmaven-surefire's IT fixture (apache/maven-surefire#3484); the fixture now creates one executor perVerifierinstance to avoid it.Proposal: take the snapshot at the start of each
execute()rather than in the constructor. This is distinct from the thread-safety window in #16 item 8, which is aboutSystem.setProperties(null)during a run.The same JVM-lifetime pattern is in
apache/mavenitself:its/core-it-support/maven-it-helper/.../Verifier.javaholdsstatic final EmbeddedMavenExecutorandForkedMavenExecutorinstances, so the core ITs on master and on the 3.x branches (apache/maven-integration-testing#445) are exposed as well; fixing the snapshot point in the executor covers all of them.The obvious workaround, one
EmbeddedMavenExecutorper execution closed afterwards, does not work at scale: on apache/maven-surefire#3484 (run 35594937350) the IT JVM runs out of heap after roughly a hundred builds (java.lang.OutOfMemoryError: Java heap spaceinside the embedded Maven), becauseclose()does not release the Maven ClassWorld the constructor built. So callers are pushed back to a JVM-lifetime executor and inherit the snapshot behaviour; moving the snapshot intoexecute()is the fix that avoids both.