diff --git a/src/jdk.jpackage/windows/native/applauncher/WinLauncher.cpp b/src/jdk.jpackage/windows/native/applauncher/WinLauncher.cpp index b81dc27d278..407cf00693b 100644 --- a/src/jdk.jpackage/windows/native/applauncher/WinLauncher.cpp +++ b/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(); - 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(exec.execAndWaitForExit()); exit(exitCode); return; diff --git a/src/jdk.jpackage/windows/native/msiwrapper/Executor.cpp b/src/jdk.jpackage/windows/native/common/Executor.cpp similarity index 71% rename from src/jdk.jpackage/windows/native/msiwrapper/Executor.cpp rename to src/jdk.jpackage/windows/native/common/Executor.cpp index 5535d81b482..edb850afdbb 100644 --- a/src/jdk.jpackage/windows/native/msiwrapper/Executor.cpp +++ b/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 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); diff --git a/src/jdk.jpackage/windows/native/msiwrapper/Executor.h b/src/jdk.jpackage/windows/native/common/Executor.h similarity index 76% rename from src/jdk.jpackage/windows/native/msiwrapper/Executor.h rename to src/jdk.jpackage/windows/native/common/Executor.h index 72f33db57b5..a6edcbd4f76 100644 --- a/src/jdk.jpackage/windows/native/msiwrapper/Executor.h +++ b/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; }; diff --git a/test/jdk/tools/jpackage/windows/Win8301247Test.java b/test/jdk/tools/jpackage/windows/Win8301247Test.java new file mode 100644 index 00000000000..67163ee5c60 --- /dev/null +++ b/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 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 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("^(?\\d+)\\s+(?\\d+)\\s+$"); + } else if (headers[1].equals("parentprocessid") && headers[0].equals( + "processid")) { + pattern = Pattern.compile("^(?\\d+)\\s+(?\\d+)\\s+$"); + } else { + throw new RuntimeException( + "Unrecognizable output of \'wmic process\' command"); + } + + List 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(); + } + } +}