Skip to content

Commit 0b19014

Browse files
olamyolamy
authored
Replace runing external process and parsing output with simple ProcessHandle if available (Java9+) (#3252)
* Replacing runing external process and parsing output with simple ProcessHandle if available Signed-off-by: Olivier Lamy <olamy@apache.org>
1 parent 688f8c4 commit 0b19014

13 files changed

Lines changed: 851 additions & 452 deletions

File tree

‎maven-surefire-common/pom.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@
172172
<dependency>
173173
<groupId>commons-io</groupId>
174174
<artifactId>commons-io</artifactId>
175-
<version>2.21.0</version>
176175
<scope>test</scope>
177176
</dependency>
178177
</dependencies>

‎maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/Platform.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929

3030
/**
3131
* Loads platform specifics.
32-
*
32+
* TODO simplify or remove when Java 8 support is dropped
3333
* @author <a href="mailto:tibordigana@apache.org">Tibor Digana (tibor17)</a>
3434
* @since 2.20.1
3535
*/
@@ -80,11 +80,6 @@ public Platform withJdkExecAttributesForTests(JdkAttributes jdk) {
8080
}
8181

8282
private static Callable<Long> pidJob() {
83-
return new Callable<Long>() {
84-
@Override
85-
public Long call() throws Exception {
86-
return SystemUtils.pid();
87-
}
88-
};
83+
return SystemUtils::pid;
8984
}
9085
}

‎maven-surefire-plugin/src/site/apt/examples/shutdown.apt.vm‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,13 @@ Shutdown of Forked JVM
5555

5656
[]
5757

58+
If Java9 is available, the start time of the process is determined by <<< ProcessHandle.current().info().startInstant() >>>.
59+
5860
On Unix like systems the process' uptime is determined by native command <<< (/usr)/bin/ps -o etime= -p [PID] >>>.
5961

60-
On Windows the start time is determined using <<< wmic process where (ProcessId=[PID]) get CreationDate >>>
62+
On Windows the start time is determined using <<< powershell -command "... Get-CimInstance Win32_Process ..." >>>.
63+
64+
[]
6165
in the forked JVM.
6266

6367

‎surefire-booter/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@
9191
<artifactId>powermock-api-mockito2</artifactId>
9292
<scope>test</scope>
9393
</dependency>
94+
<dependency>
95+
<groupId>commons-io</groupId>
96+
<artifactId>commons-io</artifactId>
97+
<scope>test</scope>
98+
</dependency>
9499
</dependencies>
95100

96101
<build>

‎surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ private void closeForkChannel() {
215215
}
216216

217217
private PingScheduler listenToShutdownCommands(String ppid) {
218-
PpidChecker ppidChecker = ppid == null ? null : new PpidChecker(ppid);
218+
ProcessChecker ppidChecker = ProcessChecker.of(ppid);
219219
commandReader.addShutdownListener(createExitHandler(ppidChecker));
220220
AtomicBoolean pingDone = new AtomicBoolean(true);
221221
commandReader.addNoopListener(createPingHandler(pingDone));
@@ -280,7 +280,7 @@ public void update(Command command) {
280280
};
281281
}
282282

283-
private CommandListener createExitHandler(final PpidChecker ppidChecker) {
283+
private CommandListener createExitHandler(final ProcessChecker ppidChecker) {
284284
return new CommandListener() {
285285
@Override
286286
public void update(Command command) {
@@ -325,7 +325,7 @@ public void update(Command command) {
325325
};
326326
}
327327

328-
private Runnable createPingJob(final AtomicBoolean pingDone, final PpidChecker pluginProcessChecker) {
328+
private Runnable createPingJob(final AtomicBoolean pingDone, final ProcessChecker pluginProcessChecker) {
329329
return new Runnable() {
330330
@Override
331331
public void run() {
@@ -515,7 +515,7 @@ private static void run(ForkedBooter booter, String[] args) {
515515
}
516516
}
517517

518-
private static boolean canUseNewPingMechanism(PpidChecker pluginProcessChecker) {
518+
private static boolean canUseNewPingMechanism(ProcessChecker pluginProcessChecker) {
519519
return pluginProcessChecker != null && pluginProcessChecker.canUse();
520520
}
521521

@@ -553,12 +553,12 @@ private static boolean isDebugging() {
553553
private static class PingScheduler {
554554
private final ScheduledExecutorService pingScheduler;
555555
private final ScheduledExecutorService processCheckerScheduler;
556-
private final PpidChecker processChecker;
556+
private final ProcessChecker processChecker;
557557

558558
PingScheduler(
559559
ScheduledExecutorService pingScheduler,
560560
ScheduledExecutorService processCheckerScheduler,
561-
PpidChecker processChecker) {
561+
ProcessChecker processChecker) {
562562
this.pingScheduler = pingScheduler;
563563
this.processCheckerScheduler = processCheckerScheduler;
564564
this.processChecker = processChecker;

‎surefire-booter/src/main/java/org/apache/maven/surefire/booter/PpidChecker.java‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,18 @@
5757

5858
/**
5959
* Recognizes PID of Plugin process and determines lifetime.
60+
* <p>
61+
* This implementation uses native commands ({@code ps} on Unix, {@code powershell} on Windows)
62+
* to check the parent process status. On Java 9+, consider using {@code ProcessHandleChecker}
63+
* instead, which uses the Java {@code ProcessHandle} API and doesn't require spawning external processes.
6064
*
6165
* @author <a href="mailto:tibordigana@apache.org">Tibor Digana (tibor17)</a>
6266
* @since 2.20.1
67+
* @see ProcessChecker
68+
* @deprecated Use {@code ProcessHandleChecker} via {@link ProcessChecker#of(String)} instead
6369
*/
64-
final class PpidChecker {
70+
@Deprecated
71+
final class PpidChecker implements ProcessChecker {
6572
private static final long MINUTES_TO_MILLIS = 60L * 1000L;
6673
// 25 chars https://superuser.com/questions/937380/get-creation-time-of-file-in-milliseconds/937401#937401
6774
private static final int WMIC_CREATION_DATE_VALUE_LENGTH = 25;
@@ -95,7 +102,8 @@ final class PpidChecker {
95102
this.ppid = ppid;
96103
}
97104

98-
boolean canUse() {
105+
@Override
106+
public boolean canUse() {
99107
if (isStopped()) {
100108
return false;
101109
}
@@ -111,7 +119,8 @@ boolean canUse() {
111119
* or this object has been {@link #destroyActiveCommands() destroyed}
112120
* @throws NullPointerException if extracted e-time is null
113121
*/
114-
boolean isProcessAlive() {
122+
@Override
123+
public boolean isProcessAlive() {
115124
if (!canUse()) {
116125
throw new IllegalStateException("irrelevant to call isProcessAlive()");
117126
}
@@ -226,14 +235,16 @@ ProcessInfo consumeLine(String line, ProcessInfo previousProcessInfo) throws Exc
226235
return reader.execute(psPath + "powershell", "-NoProfile", "-NonInteractive", "-Command", psCommand);
227236
}
228237

229-
void destroyActiveCommands() {
238+
@Override
239+
public void destroyActiveCommands() {
230240
stopped = true;
231241
for (Process p = destroyableCommands.poll(); p != null; p = destroyableCommands.poll()) {
232242
p.destroy();
233243
}
234244
}
235245

236-
boolean isStopped() {
246+
@Override
247+
public boolean isStopped() {
237248
return stopped;
238249
}
239250

@@ -325,10 +336,16 @@ private static SimpleDateFormat createWindowsCreationDateFormat() {
325336
return formatter;
326337
}
327338

339+
@Override
328340
public void stop() {
329341
stopped = true;
330342
}
331343

344+
@Override
345+
public ProcessInfo processInfo() {
346+
return parentProcessInfo;
347+
}
348+
332349
/**
333350
* Reads standard output from {@link Process}.
334351
* <br>

‎surefire-booter/src/main/java/org/apache/maven/surefire/booter/ProcessChecker.java‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.maven.surefire.booter;
20+
21+
/**
22+
* Interface for checking if a process (typically the parent Maven plugin) is still alive.
23+
* <p>
24+
* Implementations allow the forked JVM to detect when its parent Maven process
25+
* has terminated, enabling cleanup and preventing orphaned processes.
26+
*
27+
* @since 3.5.5
28+
*/
29+
public interface ProcessChecker {
30+
31+
/**
32+
* Creates the appropriate {@link ProcessChecker} implementation for the given parent PID.
33+
* <p>
34+
* On Java 9+, uses {@code ProcessHandleChecker} which leverages the {@code ProcessHandle} API.
35+
* On Java 8, falls back to {@link PpidChecker} which uses native commands.
36+
*
37+
* @param ppid the parent process ID as a string, or {@code null}
38+
* @return a new checker instance, or {@code null} if ppid is {@code null}
39+
*/
40+
static ProcessChecker of(String ppid) {
41+
if (ppid == null) {
42+
return null;
43+
}
44+
if (ProcessHandleChecker.isAvailable()) {
45+
return new ProcessHandleChecker(ppid);
46+
}
47+
return new PpidChecker(ppid);
48+
}
49+
50+
/**
51+
* Returns whether the ProcessHandle API is available in the current JVM.
52+
*
53+
* @return {@code true} if running on Java 9+ with ProcessHandle available
54+
*/
55+
static boolean isProcessHandleSupported() {
56+
return ProcessHandleChecker.isAvailable();
57+
}
58+
59+
/**
60+
* Checks whether this checker can be used to monitor the process.
61+
* <p>
62+
* This method must return {@code true} before {@link #isProcessAlive()} can be called.
63+
* @deprecated with using ProcessHandleChecker on Java 9+, this method will always return {@code true} and can be removed in a future release.
64+
* @return {@code true} if the checker is operational and can monitor the process
65+
*/
66+
@Deprecated
67+
boolean canUse();
68+
69+
/**
70+
* Checks if the process is still alive.
71+
* <p>
72+
* This method can only be called after {@link #canUse()} has returned {@code true}.
73+
*
74+
* @return {@code true} if the process is still running; {@code false} if it has terminated
75+
* or if the PID has been reused by a different process
76+
* @throws IllegalStateException if {@link #canUse()} returns {@code false} or if the checker
77+
* has been stopped
78+
*/
79+
boolean isProcessAlive();
80+
81+
/**
82+
* Stops the checker and releases any resources.
83+
* <p>
84+
* After calling this method, {@link #canUse()} will return {@code false}.
85+
*/
86+
void stop();
87+
88+
/**
89+
* Destroys any active commands or subprocesses used by this checker.
90+
* <p>
91+
* This is called during shutdown to ensure clean termination.
92+
*/
93+
void destroyActiveCommands();
94+
95+
/**
96+
* Checks if the checker has been stopped.
97+
*
98+
* @return {@code true} if {@link #stop()} or {@link #destroyActiveCommands()} has been called
99+
*/
100+
boolean isStopped();
101+
102+
/**
103+
* Returns information about the process being checked.
104+
*
105+
* @return the process information, or {@code null} if not yet initialized
106+
*/
107+
ProcessInfo processInfo();
108+
}

0 commit comments

Comments
 (0)