8301247: JPackage app-image exe launches multiple exe's in JDK 17+

Reviewed-by: almatvee

src/jdk.jpackage/windows/native/applauncher/WinLauncher.cpp

@@ -34,6 +34,7 @@
 #include "Dll.h"
 #include "WinApp.h"
 #include "Toolbox.h"
+#include "Executor.h"
 #include "FileUtils.h"
 #include "PackageFile.h"
 #include "UniqueHandle.h"
@@ -180,29 +181,29 @@ void launchApp() {
 
         jvm = std::unique_ptr<Jvm>();
 
-        STARTUPINFOW si;
-        ZeroMemory(&si, sizeof(si));
-        si.cb = sizeof(si);
-
-        PROCESS_INFORMATION pi;
-        ZeroMemory(&pi, sizeof(pi));
-
-        if (!CreateProcessW(launcherPath.c_str(), GetCommandLineW(),
-                NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) {
-            JP_THROW(SysError(tstrings::any() << "CreateProcessW() failed",
-                                                            CreateProcessW));
+        UniqueHandle jobHandle(CreateJobObject(NULL, NULL));
+        if (jobHandle.get() == NULL) {
+            JP_THROW(SysError(tstrings::any() << "CreateJobObject() failed",
+                                                            CreateJobObject));
+        }
+        JOBOBJECT_EXTENDED_LIMIT_INFORMATION jobInfo = { };
+        jobInfo.BasicLimitInformation.LimitFlags =
+                                          JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
+        if (!SetInformationJobObject(jobHandle.get(),
+                JobObjectExtendedLimitInformation, &jobInfo, sizeof(jobInfo))) {
+            JP_THROW(SysError(tstrings::any() <<
+                                            "SetInformationJobObject() failed",
+                                                    SetInformationJobObject));
         }
 
-        WaitForSingleObject(pi.hProcess, INFINITE);
+        Executor exec(launcherPath);
+        exec.visible(true).withJobObject(jobHandle.get()).suspended(true).inherit(true);
+        const auto args = SysInfo::getCommandArgs();
+        std::for_each(args.begin(), args.end(), [&exec] (const tstring& arg) {
+            exec.arg(arg);
+        });
 
-        UniqueHandle childProcessHandle(pi.hProcess);
-        UniqueHandle childThreadHandle(pi.hThread);
-
-        DWORD exitCode;
-        if (!GetExitCodeProcess(pi.hProcess, &exitCode)) {
-            JP_THROW(SysError(tstrings::any() << "GetExitCodeProcess() failed",
-                                                        GetExitCodeProcess));
-        }
+        DWORD exitCode = static_cast<DWORD>(exec.execAndWaitForExit());
 
         exit(exitCode);
         return;

src/jdk.jpackage/windows/native/common/Executor.cpp

@@ -60,7 +60,15 @@ std::wstring Executor::args() const {
 
 
 int Executor::execAndWaitForExit() const {
-    UniqueHandle h = startProcess();
+    UniqueHandle threadHandle;
+    UniqueHandle h = startProcess(&threadHandle);
+
+    if (theSuspended) {
+        LOG_TRACE(tstrings::any() << "ResumeThread()");
+        if (((DWORD)-1) == ResumeThread(threadHandle.get())) {
+            JP_THROW(SysError("ResumeThread() failed", ResumeThread));
+        }
+    }
 
     const DWORD res = ::WaitForSingleObject(h.get(), INFINITE);
     if (WAIT_FAILED ==  res) {
@@ -85,7 +93,7 @@ int Executor::execAndWaitForExit() const {
 }
 
 
-UniqueHandle Executor::startProcess() const {
+UniqueHandle Executor::startProcess(UniqueHandle* threadHandle) const {
     const std::wstring argsStr = args();
 
     std::vector<TCHAR> argsBuffer(argsStr.begin(), argsStr.end());
@@ -100,6 +108,10 @@ UniqueHandle Executor::startProcess() const {
 
     DWORD creationFlags = 0;
 
+    if (theSuspended) {
+        creationFlags |= CREATE_SUSPENDED;
+    }
+
     if (!theVisible) {
         // For GUI applications.
         startupInfo.dwFlags |= STARTF_USESHOWWINDOW;
@@ -110,10 +122,21 @@ UniqueHandle Executor::startProcess() const {
     }
 
     tstrings::any msg;
-    msg << "CreateProcess(" << appPath << ", " << argsStr << ")";
+    msg << "CreateProcess";
+    if (theSuspended) {
+        msg << "[suspended]";
+    }
+    if (theVisible) {
+        msg << "[visible]";
+    }
+    if (theInherit) {
+        msg << "[inherit]";
+    }
+    msg << "(" << appPath << ", " << argsStr << ")";
 
-    if (!CreateProcess(appPath.c_str(), argsBuffer.data(), NULL, NULL, FALSE,
-                    creationFlags, NULL, NULL, &startupInfo, &processInfo)) {
+    if (!CreateProcess(appPath.c_str(), argsBuffer.data(), NULL, NULL,
+                      theInherit ? TRUE : FALSE, creationFlags, NULL, NULL,
+                      &startupInfo, &processInfo)) {
         msg << " failed";
         JP_THROW(SysError(msg, CreateProcess));
     }
@@ -121,8 +144,22 @@ UniqueHandle Executor::startProcess() const {
     msg << " succeeded; PID=" << processInfo.dwProcessId;
     LOG_TRACE(msg);
 
-    // Close unneeded handles immediately.
-    UniqueHandle(processInfo.hThread);
+    if (threadHandle) {
+        *threadHandle = UniqueHandle(processInfo.hThread);
+    } else {
+        // Close unneeded handle immediately.
+        UniqueHandle(processInfo.hThread);
+    }
+
+    if (jobHandle != NULL) {
+        LOG_TRACE(tstrings::any() << "AssignProcessToJobObject(PID="
+                << processInfo.dwProcessId << ")");
+        if (!AssignProcessToJobObject(jobHandle, processInfo.hProcess)) {
+            JP_THROW(SysError(tstrings::any() <<
+                    "AssignProcessToJobObject() failed",
+                    AssignProcessToJobObject));
+        }
+    }
 
     // Return process handle.
     return UniqueHandle(processInfo.hProcess);

src/jdk.jpackage/windows/native/common/Executor.h

@@ -33,7 +33,7 @@
 class Executor {
 public:
     explicit Executor(const std::wstring& appPath=std::wstring()) {
-        app(appPath).visible(false);
+        app(appPath).visible(false).suspended(false).withJobObject(NULL).inherit(false);
     }
 
     /**
@@ -65,6 +65,30 @@ public:
         return *this;
     }
 
+    /**
+     * Controls if the process should inherit handles.
+     */
+    Executor& inherit(bool v) {
+        theInherit = v;
+        return *this;
+    }
+
+    /**
+     * Controls if the process should be started suspended.
+     */
+    Executor& suspended(bool v) {
+        theSuspended = v;
+        return *this;
+    }
+
+    /**
+     * Use the given job object with started process.
+     */
+    Executor& withJobObject(HANDLE v) {
+        jobHandle = v;
+        return *this;
+    }
+
     /**
      * Starts application process and blocks waiting when the started
      * process terminates.
@@ -74,9 +98,12 @@ public:
     int execAndWaitForExit() const;
 
 private:
-    UniqueHandle startProcess() const;
+    UniqueHandle startProcess(UniqueHandle* threadHandle=0) const;
 
     bool theVisible;
+    bool theInherit;
+    bool theSuspended;
+    HANDLE jobHandle;
     tstring_array argsArray;
     std::wstring appPath;
 };

test/jdk/tools/jpackage/windows/Win8301247Test.java

@@ -0,0 +1,152 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Stream;
+import jdk.jpackage.test.JPackageCommand;
+import jdk.jpackage.test.Annotations.Test;
+import jdk.jpackage.test.Executor;
+import jdk.jpackage.test.HelloApp;
+import jdk.jpackage.test.TKit;
+
+/**
+ * Test that terminating of the parent app launcher process automatically
+ * terminates child app launcher process.
+ */
+
+/*
+ * @test
+ * @summary Test case for JDK-8301247
+ * @library ../helpers
+ * @build jdk.jpackage.test.*
+ * @build Win8301247Test
+ * @requires (os.family == "windows")
+ * @modules jdk.jpackage/jdk.jpackage.internal
+ * @run main/othervm/timeout=360 -Xmx512m  jdk.jpackage.test.Main
+ *  --jpt-run=Win8301247Test
+ */
+public class Win8301247Test {
+
+    @Test
+    public void test() throws IOException, InterruptedException {
+        JPackageCommand cmd = JPackageCommand.helloAppImage();
+
+        // Launch the app in a way it doesn't exit to let us trap app laucnher
+        // processes in the process list
+        cmd.addArguments("--java-options", "-Djpackage.test.noexit=true");
+        cmd.executeAndAssertImageCreated();
+
+        if (!cmd.canRunLauncher("Not running the test")) {
+            return;
+        }
+
+        try ( // Launch the app in a separate thread
+                ExecutorService exec = Executors.newSingleThreadExecutor()) {
+            exec.execute(() -> {
+                HelloApp.executeLauncher(cmd);
+            });
+
+            // Wait a bit to let the app start
+            Thread.sleep(Duration.ofSeconds(10));
+
+            // Get PID of the main app launcher process
+            final long pid = findMainAppLauncherPID(cmd, 2).get();
+
+            // Kill the main app launcher process
+            Executor.of("taskkill", "/F", "/PID", Long.toString(pid)).
+                    dumpOutput(true).execute();
+
+            // Wait a bit and check if child app launcher process is still running (it must NOT)
+            Thread.sleep(Duration.ofSeconds(5));
+
+            findMainAppLauncherPID(cmd, 0);
+        }
+    }
+
+    private static Optional<Long> findMainAppLauncherPID(JPackageCommand cmd,
+            int expectedCount) {
+        // Get the list of PIDs and PPIDs of app launcher processes.
+        // wmic process where (name = "foo.exe") get ProcessID,ParentProcessID
+        List<String> output = Executor.of("wmic", "process", "where", "(name",
+                "=",
+                "\"" + cmd.appLauncherPath().getFileName().toString() + "\"",
+                ")", "get", "ProcessID,ParentProcessID").dumpOutput(true).
+                saveOutput().executeAndGetOutput();
+
+        if (expectedCount == 0) {
+            TKit.assertEquals("No Instance(s) Available.", output.getFirst().
+                    trim(), "Check no app launcher processes found running");
+            return Optional.empty();
+        }
+
+        String[] headers = Stream.of(output.getFirst().split("\\s+", 2)).map(
+                String::trim).map(String::toLowerCase).toArray(String[]::new);
+        Pattern pattern;
+        if (headers[0].equals("parentprocessid") && headers[1].equals(
+                "processid")) {
+            pattern = Pattern.compile("^(?<ppid>\\d+)\\s+(?<pid>\\d+)\\s+$");
+        } else if (headers[1].equals("parentprocessid") && headers[0].equals(
+                "processid")) {
+            pattern = Pattern.compile("^(?<pid>\\d+)\\s+(?<ppid>\\d+)\\s+$");
+        } else {
+            throw new RuntimeException(
+                    "Unrecognizable output of \'wmic process\' command");
+        }
+
+        List<long[]> processes = output.stream().skip(1).map(line -> {
+            Matcher m = pattern.matcher(line);
+            long[] pids = null;
+            if (m.matches()) {
+                pids = new long[]{Long.parseLong(m.group("pid")), Long.
+                    parseLong(m.group("ppid"))};
+            }
+            return pids;
+        }).filter(Objects::nonNull).toList();
+
+        TKit.assertEquals(expectedCount, processes.size(), String.format(
+                "Check [%d] app launcher processes found running", expectedCount));
+
+        switch (expectedCount) {
+            case 2 -> {
+                if (processes.get(0)[0] == processes.get(1)[1]) {
+                    return Optional.of(processes.get(0)[0]);
+                } else if (processes.get(1)[0] == processes.get(0)[1]) {
+                    return Optional.of(processes.get(1)[0]);
+                } else {
+                    throw new RuntimeException(
+                            "App launcher processes unrelated");
+                }
+            }
+            default ->
+                throw new IllegalArgumentException();
+        }
+    }
+}