Skip to content

Replace runing external process and parsing output with simple ProcessHandle if available (Java9+)#3252

Merged
olamy merged 24 commits intoapache:masterfrom
olamy:ppidchecker-processhandle
Feb 10, 2026
Merged

Replace runing external process and parsing output with simple ProcessHandle if available (Java9+)#3252
olamy merged 24 commits intoapache:masterfrom
olamy:ppidchecker-processhandle

Conversation

@olamy
Copy link
Member

@olamy olamy commented Feb 4, 2026

Signed-off-by: Olivier Lamy olamy@apache.org

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the integration tests successfully (mvn -Prun-its clean install).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

@olamy
Copy link
Member Author

olamy commented Feb 4, 2026

tentative option for #3176

@olamy olamy added the enhancement New feature or request label Feb 5, 2026
@olamy olamy force-pushed the ppidchecker-processhandle branch from 66b9a60 to 15a377e Compare February 5, 2026 03:06
@olamy olamy changed the title [WIP] Try replacing runing external process and parsing output with simple ProcessHandle if available [WIP] Replace runing external process and parsing output with simple ProcessHandle if available Feb 5, 2026
@olamy olamy marked this pull request as ready for review February 5, 2026 11:38
@olamy olamy changed the title [WIP] Replace runing external process and parsing output with simple ProcessHandle if available Replace runing external process and parsing output with simple ProcessHandle if available (Java9+) Feb 5, 2026
@olamy olamy added this to the 3.5.5 milestone Feb 5, 2026
@olamy olamy force-pushed the ppidchecker-processhandle branch from 4a2eb85 to 57029db Compare February 5, 2026 21:03
timeout-minutes: 600
os-matrix: '[ "ubuntu-latest", "windows-2022", "macos-latest" ]'
os-matrix: '[ "ubuntu-latest", "windows-2022", "windows-latest", "macos-latest" ]'
# exclude jkd 8 on windows-latest wmc process not here anymore and jdk8 not supporting processhandle
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does mean that surefire will not works on JDK8 at windows?

Copy link
Member Author

@olamy olamy Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's already the case (not for gh node windows-latest with Java 8) see #3176
and not much we can do except using some other external tool such as proposed here #3176 (comment) but I find too complicated as it will be needed only for Java8

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So when this breaks execution of surefire on JDK 8, this is a PR for 4.x.x (and Java 17)?

Copy link
Member Author

@olamy olamy Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So when this breaks execution of surefire on JDK 8, this is a PR for 4.x.x (and Java 17)?

nope. still working on jdk8 for windows-2022, linux or osx.
exclusion is ONLY windows-latest and jdk8. but now will work on windows-latest for modern jdk (9+ as it's not working now)
It's only broken now for jdk8 on windows-latest but will be find for jdk9+ please read linked issue and the comment added in maven-verify.yml
Sorry I don't really want to spend time to fix for windows-latest and jdk8

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update to:

      os-matrix: '[ "ubuntu-latest", "windows-latest", "macos-latest" ]'
      matrix-exclude: >
        [ 
          {"jdk": "8", "os": "windows-latest"}
        ]
      matrix-include: >
        [
          {"jdk": "8", "os": "windows-2022", "distribution": "zulu", "maven": "3.9.12"}
        ]

we will have execution - windows-latest for all jdk expect jdk8 and windows-2022 only for jdk8

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you add windows-2022 and windows-latest to global matrix with one exclude, we will have executed

windows-2022 - jdk8, jdk11, jdk17, jdk21
windows-latest -  jdk11, jdk17, jdk21

so will be a 7 builds for windows

with my proposition we will have:

windows-2022 - jdk8
windows-latest -  jdk11, jdk17, jdk21

4 builds

I'm not sure if we need such many windows builds

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, good point.
Maybe another build on windows-2022 with a more recent JDK?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe another build on windows-2022 with a more recent JDK?

JDK21 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slawekjaranowski should be better now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if #3258 is accepted, there is no need to do any change in the matrix

@olamy olamy force-pushed the ppidchecker-processhandle branch from 57029db to 1e3a1ca Compare February 6, 2026 00:03
@Tibor17
Copy link
Contributor

Tibor17 commented Feb 6, 2026

It does not make sense to support Java 8 and Java 9. We should switch to Java 9 only.

@Tibor17
Copy link
Contributor

Tibor17 commented Feb 6, 2026

I have a strong reason to forget java 8 in Surefire because the PPID Checker needs to use java 9.
Currently we are calling a native Windows command which does not exist in Windows anymore. And it does not make sense to me to support both versions - crazy!!!

@slachiewicz
Copy link
Member

I have a strong reason to forget java 8 in Surefire because the PPID Checker needs to use java 9. Currently we are calling a native Windows command which does not exist in Windows anymore. And it does not make sense to me to support both versions - crazy!!!

This sould be discused on mailing list - to drop Java 8 support

Copy link

@bmarwell bmarwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, because JDK9+ on latest windows will start working again, so it's better than doing nothing.

olamy added 18 commits February 10, 2026 07:26
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
…rue with Java9+ and ProcessHandle

Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy olamy force-pushed the ppidchecker-processhandle branch from e2126a3 to a280d15 Compare February 9, 2026 21:29
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
@jbliznak
Copy link
Contributor

The last thing seems to be an update of https://github.com/apache/maven-surefire/blob/master/maven-surefire-plugin/src/site/apt/examples/shutdown.apt.vm#L58-L61

I should have updated it with wmic->powershell change in #3258 but it needs more info after this PR anyway.

Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
Copy link
Member Author

olamy commented Feb 10, 2026

The last thing seems to be an update of https://github.com/apache/maven-surefire/blob/master/maven-surefire-plugin/src/site/apt/examples/shutdown.apt.vm#L58-L61

I should have updated it with wmic->powershell change in #3258 but it needs more info after this PR anyway.

Good catch. I have updated the page. Not with too many details, as this will be gone rather sooner than later.

@olamy olamy merged commit 0b19014 into apache:master Feb 10, 2026
14 checks passed
@olamy olamy deleted the ppidchecker-processhandle branch February 10, 2026 09:08
@Tibor17
Copy link
Contributor

Tibor17 commented Feb 14, 2026

@olamy @slachiewicz I will have a look into these changes. But I agree to discuss it on the mailing list. Of course, there's a compromise. There are several notices on my side:

  1. We have also customers with old Windows systems: Windows XP, Windows Server.
  2. It is not true that the PowerShell is the only solution because not everybody wants to install it nor installed it yet.
  3. It is true that using OS-naturally native commands we are significantly mitigating the risk (the risk that some native binary is not recognized).
  4. It is true that the ProcessHandle is totally avoiding this issue. Unlikely the PS or WMIC or PowerShell commands which are only at the mitigation level - there is always some risk but it is minimum.
  5. It is true that using a kind of Java reflection (or a modern MethodHandle) we can call ProcessHandle without any problem even on Java 8 build process of this project (customer's build process is different story - there ProcessHandle can be activated).
  6. If we do not want to switch to Java 9, I am fine with that, but then we have to continue in the risk mitigation and we should not push the customer to something he does not like and it is reinstallation of old systems with PowerShell (which might not be installable). In this case adding PowerShell is okay but we have to keep WMIC for old Windows systems, and PS for *Nix systems. Due to the Java 9 API is simple, and it is simple to call (in principal) ProcessHandle.of(PPID).isAlive() via the reflection - we have these experiences, we can switch from risk mitigation to a guarantee. Since of Maven 4, we can delete all of these PowerShell, WMIC, PS commands and call ProcessHandle.of(PPID).isAlive() without reflection - trivial.
  7. Additionally, nobody has mentioned that there is one more problem. Nobody would recognize this issue if the native commands fallbacked to the PING mechanism. There is a bug, and it does not matter if you add PowerShell or the trick with Java 9. It does not matter! This is missing, still.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Surefire and Failsafe stop working on latest versions of Windows due to missing wmic

9 participants