RFR: 8340311: JPackage app-image exe launches multiple exe's in JDK 22+ [v2]

Alexander Matveev almatvee at openjdk.org
Thu Oct 31 22:36:31 UTC 2024


On Thu, 31 Oct 2024 05:52:07 GMT, Alexey Semenyuk <asemenyuk at openjdk.org> wrote:

>> Support `win.norestart` application property in jpackage .cfg file. If found in the .cfg file and set to `true`, it forces the Windows app launcher not to restart the launcher process.
>> 
>> Collateral changes to the tests:
>>  - Added `WindowsHelper.killProcess()` to kill process by PID;
>>  - Added `WindowsHelper.findAppLauncherPID()` that will return PIDs of the running app launcher processes for the given app launcher name. The implementation grabbed from `Win8301247Test` test;
>>  - Rework `CfgFile` to make it R/W and capable of writing multiple instances of the same key;
>>  - Added `JPackageCommand.ignoreFakeRuntime()` to ignore external runtime if it is a stub. Should speed up local test runs.
>
> Alexey Semenyuk has updated the pull request incrementally with four additional commits since the last revision:
> 
>  - Better log message
>  - Rework findAppLauncherPID() into findAndKillAppLauncherProcess() that will kill detected running app launcher processes regardless if their number is as expected or not.
>  - A comment added
>  - Fix copyright year

Looks good overall with some additional comments.

src/jdk.jpackage/share/native/applauncher/AppLauncher.cpp line 41:

> 39:     launcherPath = SysInfo::getProcessModulePath();
> 40:     args = SysInfo::getCommandArgs();
> 41:     externalCfgFile = 0;

I would prefer `NULL` instead of `0`.

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java line 235:

> 233: 
> 234:     public static void killProcess(long pid) {
> 235:         Executor.of("taskkill", "/F", "/PID", Long.toString(pid)).dumpOutput(true).execute();

In case of two processes (parent and child) it will terminate only parent. Based on `taskkill /?` `/T` needs to be specified to terminate child process as well or call it for both processes.

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java line 311:

> 309:             default -> {
> 310:                 TKit.assertUnexpected(String.format(
> 311:                         "Unexpected numer of running processes [%d]",

`numer` -> `number`

-------------

PR Review: https://git.openjdk.org/jdk/pull/21726#pullrequestreview-2409202951
PR Review Comment: https://git.openjdk.org/jdk/pull/21726#discussion_r1825242220
PR Review Comment: https://git.openjdk.org/jdk/pull/21726#discussion_r1825248460
PR Review Comment: https://git.openjdk.org/jdk/pull/21726#discussion_r1825247266


More information about the core-libs-dev mailing list