Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v6]
On Fri, 5 Feb 2021 17:44:21 GMT, Alexey Semenyuk wrote: >> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 >> >> The fix splits Linux app launcher in app launcher and launcher shared lib. >> App launcher is pure C and doesn't have C++ code. App launcher lib >> incorporates bulk of C++ code from app launcher. >> At startup app launcher loads launcher shared lib and calls functions it >> provides to get data for launching JVM (path to jli lib and arguments for >> JLI_Launch function call). >> App launcher unloads launcher shared lib before launching JVM to remove C++ >> runtime from the process memory. >> >> Getting rid of C++ code from app launcher required to rewrite app >> installation location lookup code from C++ to C. LinuxPackage.c source is C >> alternative for >> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp >> and >> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. >> >> Layout of jpackage's native code changed: >> - `common`: code shared between all jpackage binaries. >> - `applauncher`: launcher only code. >> - `applauncherlib`: launcher lib code on Linux and launcher code on other >> platforms. >> - `applaunchercommon`: code shared between launcher and launcher lib on >> Linux and launcher code on other platforms. > > Alexey Semenyuk has updated the pull request incrementally with one > additional commit since the last revision: > > 8254702: jpackage app launcher crashes on CentOS looks ok if build team approves - Marked as reviewed by herrick (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v6]
On Fri, 5 Feb 2021 17:44:21 GMT, Alexey Semenyuk wrote: >> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 >> >> The fix splits Linux app launcher in app launcher and launcher shared lib. >> App launcher is pure C and doesn't have C++ code. App launcher lib >> incorporates bulk of C++ code from app launcher. >> At startup app launcher loads launcher shared lib and calls functions it >> provides to get data for launching JVM (path to jli lib and arguments for >> JLI_Launch function call). >> App launcher unloads launcher shared lib before launching JVM to remove C++ >> runtime from the process memory. >> >> Getting rid of C++ code from app launcher required to rewrite app >> installation location lookup code from C++ to C. LinuxPackage.c source is C >> alternative for >> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp >> and >> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. >> >> Layout of jpackage's native code changed: >> - `common`: code shared between all jpackage binaries. >> - `applauncher`: launcher only code. >> - `applauncherlib`: launcher lib code on Linux and launcher code on other >> platforms. >> - `applaunchercommon`: code shared between launcher and launcher lib on >> Linux and launcher code on other platforms. > > Alexey Semenyuk has updated the pull request incrementally with one > additional commit since the last revision: > > 8254702: jpackage app launcher crashes on CentOS I like this much better, thanks for taking the time! Build changes look ok. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]
On Mon, 1 Feb 2021 18:35:54 GMT, Alexey Semenyuk wrote: >>> "common" was perfectly enough until this change. Unfortunately we cant just >>> drop new C sources in "common" dir because we don't want them to be >>> compiled with g++. That is why need new common directory >>> (applauncherlibcommon) for C sources. >> >> We pick compiler based on file suffix, not directory, so it shouldn't matter >> where you put a .c file, it should always be compiled with gcc and .cpp >> files with g++. Which compiler is used to launch the linker can however >> differ. That's configured for each SetupNativeCompilation call with the >> different TOOLCHAIN settings. > > Erik, thank you for explanation. > > The launcher on Linux should not be linked with c++ runtime, that is why > TOOLCHAIN_DEFAULT is used as a value for TOOLCHAIN property in > BUILD_JPACKAGE_APPLAUNCHEREXE target on Linux. > > Will SetupNativeCompilation work if `TOOLCHAIN=TOOLCHAIN_DEFAULT` and there > are .cpp sources are in directories passed in `SRC` property of > SetupNativeCompilation? Will it try to compile these sources? If it will > ignore them and pick only .c files, that would be perfect. Reworked the fix to avoid creation of extra source directories and file renames. - PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v6]
> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 > > The fix splits Linux app launcher in app launcher and launcher shared lib. > App launcher is pure C and doesn't have C++ code. App launcher lib > incorporates bulk of C++ code from app launcher. > At startup app launcher loads launcher shared lib and calls functions it > provides to get data for launching JVM (path to jli lib and arguments for > JLI_Launch function call). > App launcher unloads launcher shared lib before launching JVM to remove C++ > runtime from the process memory. > > Getting rid of C++ code from app launcher required to rewrite app > installation location lookup code from C++ to C. LinuxPackage.c source is C > alternative for > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp > and > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. > > Layout of jpackage's native code changed: > - `common`: code shared between all jpackage binaries. > - `applauncher`: launcher only code. > - `applauncherlib`: launcher lib code on Linux and launcher code on other > platforms. > - `applaunchercommon`: code shared between launcher and launcher lib on Linux > and launcher code on other platforms. Alexey Semenyuk has updated the pull request incrementally with one additional commit since the last revision: 8254702: jpackage app launcher crashes on CentOS - Changes: - all: https://git.openjdk.java.net/jdk/pull/2320/files - new: https://git.openjdk.java.net/jdk/pull/2320/files/b2413570..3e73e074 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=04-05 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320 PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v5]
> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 > > The fix splits Linux app launcher in app launcher and launcher shared lib. > App launcher is pure C and doesn't have C++ code. App launcher lib > incorporates bulk of C++ code from app launcher. > At startup app launcher loads launcher shared lib and calls functions it > provides to get data for launching JVM (path to jli lib and arguments for > JLI_Launch function call). > App launcher unloads launcher shared lib before launching JVM to remove C++ > runtime from the process memory. > > Getting rid of C++ code from app launcher required to rewrite app > installation location lookup code from C++ to C. LinuxPackage.c source is C > alternative for > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp > and > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. > > Layout of jpackage's native code changed: > - `common`: code shared between all jpackage binaries. > - `applauncher`: launcher only code. > - `applauncherlib`: launcher lib code on Linux and launcher code on other > platforms. > - `applaunchercommon`: code shared between launcher and launcher lib on Linux > and launcher code on other platforms. Alexey Semenyuk has updated the pull request incrementally with one additional commit since the last revision: 8254702: jpackage app launcher crashes on CentOS - Changes: - all: https://git.openjdk.java.net/jdk/pull/2320/files - new: https://git.openjdk.java.net/jdk/pull/2320/files/b6c62a2a..b2413570 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=03-04 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320 PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v4]
> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 > > The fix splits Linux app launcher in app launcher and launcher shared lib. > App launcher is pure C and doesn't have C++ code. App launcher lib > incorporates bulk of C++ code from app launcher. > At startup app launcher loads launcher shared lib and calls functions it > provides to get data for launching JVM (path to jli lib and arguments for > JLI_Launch function call). > App launcher unloads launcher shared lib before launching JVM to remove C++ > runtime from the process memory. > > Getting rid of C++ code from app launcher required to rewrite app > installation location lookup code from C++ to C. LinuxPackage.c source is C > alternative for > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp > and > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. > > Layout of jpackage's native code changed: > - `common`: code shared between all jpackage binaries. > - `applauncher`: launcher only code. > - `applauncherlib`: launcher lib code on Linux and launcher code on other > platforms. > - `applaunchercommon`: code shared between launcher and launcher lib on Linux > and launcher code on other platforms. Alexey Semenyuk has updated the pull request incrementally with two additional commits since the last revision: - 8254702: jpackage app launcher crashes on CentOS - 8254702: jpackage app launcher crashes on CentOS - Changes: - all: https://git.openjdk.java.net/jdk/pull/2320/files - new: https://git.openjdk.java.net/jdk/pull/2320/files/91744255..b6c62a2a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=02-03 Stats: 49 lines in 10 files changed: 26 ins; 11 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/2320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320 PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]
On 2021-02-01 10:38, Alexey Semenyuk wrote: On Mon, 1 Feb 2021 18:24:23 GMT, Erik Joelsson wrote: "common" was perfectly enough until this change. Unfortunately we cant just drop new C sources in "common" dir because we don't want them to be compiled with g++. That is why need new common directory (applauncherlibcommon) for C sources. I'll rename applauncherlibcommon in libapplaunchercommon and applauncherlib in libapplauncher in the next commit. "common" was perfectly enough until this change. Unfortunately we cant just drop new C sources in "common" dir because we don't want them to be compiled with g++. That is why need new common directory (applauncherlibcommon) for C sources. We pick compiler based on file suffix, not directory, so it shouldn't matter where you put a .c file, it should always be compiled with gcc and .cpp files with g++. Which compiler is used to launch the linker can however differ. That's configured for each SetupNativeCompilation call with the different TOOLCHAIN settings. Erik, thank you for explanation. The launcher on Linux should not be linked with c++ runtime, that is why TOOLCHAIN_DEFAULT is used at a value for TOOLCHAIN property in BUILD_JPACKAGE_APPLAUNCHEREXE target on Linux. Will SetupNativeCompilation work if `TOOLCHAIN=TOOLCHAIN_DEFAULT` and there are .cpp sources are in directories passed in `SRC` property of SetupNativeCompilation? Will it try to compile these sources? If it will ignore them and pick only .c files, that would be perfect. SetupNativeCompilation will by default include all src files found in any directory given to SRC, recursively. You can use EXCLUDES, EXCLUDE_FILES and EXCLUDE_PATTERN to exclude files or directories from SRC. You can also use EXTRA_FILES to pick specific files outside of any directory in SRC. Sorting files in separate directories or using EXCLUDE*/EXTRA_FILES are both possible and picking the right solution is mostly down to taste. /Erik - PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]
On Mon, 1 Feb 2021 18:24:23 GMT, Erik Joelsson wrote: >> "common" was perfectly enough until this change. Unfortunately we cant just >> drop new C sources in "common" dir because we don't want them to be compiled >> with g++. That is why need new common directory (applauncherlibcommon) for C >> sources. >> >> I'll rename applauncherlibcommon in libapplaunchercommon and applauncherlib >> in libapplauncher in the next commit. > >> "common" was perfectly enough until this change. Unfortunately we cant just >> drop new C sources in "common" dir because we don't want them to be compiled >> with g++. That is why need new common directory (applauncherlibcommon) for C >> sources. > > We pick compiler based on file suffix, not directory, so it shouldn't matter > where you put a .c file, it should always be compiled with gcc and .cpp files > with g++. Which compiler is used to launch the linker can however differ. > That's configured for each SetupNativeCompilation call with the different > TOOLCHAIN settings. Erik, thank you for explanation. The launcher on Linux should not be linked with c++ runtime, that is why TOOLCHAIN_DEFAULT is used at a value for TOOLCHAIN property in BUILD_JPACKAGE_APPLAUNCHEREXE target on Linux. Will SetupNativeCompilation work if `TOOLCHAIN=TOOLCHAIN_DEFAULT` and there are .cpp sources are in directories passed in `SRC` property of SetupNativeCompilation? Will it try to compile these sources? If it will ignore them and pick only .c files, that would be perfect. - PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]
On Mon, 1 Feb 2021 16:17:35 GMT, Alexey Semenyuk wrote: > "common" was perfectly enough until this change. Unfortunately we cant just > drop new C sources in "common" dir because we don't want them to be compiled > with g++. That is why need new common directory (applauncherlibcommon) for C > sources. We pick compiler based on file suffix, not directory, so it shouldn't matter where you put a .c file, it should always be compiled with gcc and .cpp files with g++. Which compiler is used to launch the linker can however differ. That's configured for each SetupNativeCompilation call with the different TOOLCHAIN settings. - PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v3]
> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 > > The fix splits Linux app launcher in app launcher and launcher shared lib. > App launcher is pure C and doesn't have C++ code. App launcher lib > incorporates bulk of C++ code from app launcher. > At startup app launcher loads launcher shared lib and calls functions it > provides to get data for launching JVM (path to jli lib and arguments for > JLI_Launch function call). > App launcher unloads launcher shared lib before launching JVM to remove C++ > runtime from the process memory. > > Getting rid of C++ code from app launcher required to rewrite app > installation location lookup code from C++ to C. LinuxPackage.c source is C > alternative for > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp > and > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. > > Layout of jpackage's native code changed: > - `common`: code shared between all jpackage binaries. > - `applauncher`: launcher only code. > - `applauncherlib`: launcher lib code on Linux and launcher code on other > platforms. > - `applaunchercommon`: code shared between launcher and launcher lib on Linux > and launcher code on other platforms. Alexey Semenyuk has updated the pull request incrementally with one additional commit since the last revision: 8254702: jpackage app launcher crashes on CentOS - Changes: - all: https://git.openjdk.java.net/jdk/pull/2320/files - new: https://git.openjdk.java.net/jdk/pull/2320/files/b493bcfd..91744255 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=01-02 Stats: 9 lines in 13 files changed: 0 ins; 9 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320 PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]
On Mon, 1 Feb 2021 12:19:56 GMT, Magnus Ihse Bursie wrote: >> Alexey Semenyuk has refreshed the contents of this pull request, and >> previous commits have been removed. The incremental views will show >> differences compared to the previous content of the PR. The pull request >> contains one new commit since the last revision: >> >> 8254702: jpackage app launcher crashes on CentOS > > Changes requested by ihse (Reviewer). "common" was perfectly enough until this change. Unfortunately we cant just drop new C sources in "common" dir because we don't want them to be compiled with g++. That is why need new common directory (applauncherlibcommon) for C sources. I'll rename applauncherlibcommon in libapplaunchercommon and applauncherlib in libapplauncher in the next commit. - PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]
On Mon, 1 Feb 2021 12:16:09 GMT, Magnus Ihse Bursie wrote: >> Alexey Semenyuk has refreshed the contents of this pull request, and >> previous commits have been removed. The incremental views will show >> differences compared to the previous content of the PR. The pull request >> contains one new commit since the last revision: >> >> 8254702: jpackage app launcher crashes on CentOS > > make/modules/jdk.jpackage/Lib.gmk line 61: > >> 59: JPACKAGE_OUTPUT_DIR := >> $(JDK_OUTPUTDIR)/modules/$(MODULE)/jdk/jpackage/internal/resources >> 60: JPACKAGE_CXXFLAGS_windows := -EHsc -DUNICODE -D_UNICODE >> 61: JPACKAGE_CFLAGS_windows := -DUNICODE -D_UNICODE > > Why is this change modifying Windows? I thought it was supposed to be a > linux-only fix..? There is new shared JvmlLauncherLib.c file. This new make variable is to setup complier for this file on Windows. The functional change is Linux-only, however code base code reshuffled on all platforms. > make/modules/jdk.jpackage/Lib.gmk line 65: > >> 63: )) >> 64: >> 65: $(BUILD_JPACKAGE_APPLAUNCHEREXE): $(call FindLib, java.base, java) > > Why did you remove this dependency? I moved it to the bottom of the file making all artifacts produced by make/modules/jdk.jpackage/Lib.gmk depend on java.bas and java. There is `$(JPACKAGE_TARGETS): $(call FindLib, java.base, java)` at the bottom of the file. > make/modules/jdk.jpackage/Lib.gmk line 106: > >> 104: CFLAGS_linux := -Wno-format-nonliteral, \ >> 105: LDFLAGS := $(LDFLAGS_JDKLIB) \ >> 106: >> -Wl$(COMMA)--version-script=$(TOPDIR)/make/modules/$(MODULE)/applauncherlib.ld-version-script, >> \ > > We should really not be using linker scripts. I did not understand your > comment in the linker script -- was it only needed to handle your personal > build environment? If so, you need to fix your build environment instead. Yeh, I found that linker script is needed only in my local build env. I'll remove it then. - PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]
On Fri, 29 Jan 2021 23:06:20 GMT, Alexey Semenyuk wrote: >> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 >> >> The fix splits Linux app launcher in app launcher and launcher shared lib. >> App launcher is pure C and doesn't have C++ code. App launcher lib >> incorporates bulk of C++ code from app launcher. >> At startup app launcher loads launcher shared lib and calls functions it >> provides to get data for launching JVM (path to jli lib and arguments for >> JLI_Launch function call). >> App launcher unloads launcher shared lib before launching JVM to remove C++ >> runtime from the process memory. >> >> Getting rid of C++ code from app launcher required to rewrite app >> installation location lookup code from C++ to C. LinuxPackage.c source is C >> alternative for >> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp >> and >> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. >> >> Layout of jpackage's native code changed: >> - `common`: code shared between all jpackage binaries. >> - `applauncher`: launcher only code. >> - `applauncherlib`: launcher lib code on Linux and launcher code on other >> platforms. >> - `applaunchercommon`: code shared between launcher and launcher lib on >> Linux and launcher code on other platforms. > > Alexey Semenyuk has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. This whole change seems very messy to me. :-( I'm having a hard time even untangling the PR to understand what's going on. Are you creating two new directories, "applauncherlib" and "applauncherlibcommon"? First of all, for shared libraries, the norm is to have a "lib-" prefix, not a "-lib" suffix. Secondly, there is already a "common" directory, is that not enough? Changes requested by ihse (Reviewer). src/jdk.jpackage/share/native/common/app.cpp line 26: > 24: */ > 25: > 26: #include "kludge_c++11.h" The name arose my curiosity, so I had to check out the file. Now that we indeed do have C++11 in the JDK (indeed, C++14), this should perhaps be revisited? (Not as part of this PR, of course) make/modules/jdk.jpackage/Lib.gmk line 61: > 59: JPACKAGE_OUTPUT_DIR := > $(JDK_OUTPUTDIR)/modules/$(MODULE)/jdk/jpackage/internal/resources > 60: JPACKAGE_CXXFLAGS_windows := -EHsc -DUNICODE -D_UNICODE > 61: JPACKAGE_CFLAGS_windows := -DUNICODE -D_UNICODE Why is this change modifying Windows? I thought it was supposed to be a linux-only fix..? make/modules/jdk.jpackage/Lib.gmk line 65: > 63: )) > 64: > 65: $(BUILD_JPACKAGE_APPLAUNCHEREXE): $(call FindLib, java.base, java) Why did you remove this dependency? make/modules/jdk.jpackage/Lib.gmk line 106: > 104: CFLAGS_linux := -Wno-format-nonliteral, \ > 105: LDFLAGS := $(LDFLAGS_JDKLIB) \ > 106: > -Wl$(COMMA)--version-script=$(TOPDIR)/make/modules/$(MODULE)/applauncherlib.ld-version-script, > \ We should really not be using linker scripts. I did not understand your comment in the linker script -- was it only needed to handle your personal build environment? If so, you need to fix your build environment instead. - PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]
On Fri, 29 Jan 2021 23:06:20 GMT, Alexey Semenyuk wrote: >> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 >> >> The fix splits Linux app launcher in app launcher and launcher shared lib. >> App launcher is pure C and doesn't have C++ code. App launcher lib >> incorporates bulk of C++ code from app launcher. >> At startup app launcher loads launcher shared lib and calls functions it >> provides to get data for launching JVM (path to jli lib and arguments for >> JLI_Launch function call). >> App launcher unloads launcher shared lib before launching JVM to remove C++ >> runtime from the process memory. >> >> Getting rid of C++ code from app launcher required to rewrite app >> installation location lookup code from C++ to C. LinuxPackage.c source is C >> alternative for >> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp >> and >> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. >> >> Layout of jpackage's native code changed: >> - `common`: code shared between all jpackage binaries. >> - `applauncher`: launcher only code. >> - `applauncherlib`: launcher lib code on Linux and launcher code on other >> platforms. >> - `applaunchercommon`: code shared between launcher and launcher lib on >> Linux and launcher code on other platforms. > > Alexey Semenyuk has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. Marked as reviewed by almatvee (Committer). - PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]
> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 > > The fix splits Linux app launcher in app launcher and launcher shared lib. > App launcher is pure C and doesn't have C++ code. App launcher lib > incorporates bulk of C++ code from app launcher. > At startup app launcher loads launcher shared lib and calls functions it > provides to get data for launching JVM (path to jli lib and arguments for > JLI_Launch function call). > App launcher unloads launcher shared lib before launching JVM to remove C++ > runtime from the process memory. > > Getting rid of C++ code from app launcher required to rewrite app > installation location lookup code from C++ to C. LinuxPackage.c source is C > alternative for > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp > and > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. > > Layout of jpackage's native code changed: > - `common`: code shared between all jpackage binaries. > - `applauncher`: launcher only code. > - `applauncherlib`: launcher lib code on Linux and launcher code on other > platforms. > - `applaunchercommon`: code shared between launcher and launcher lib on Linux > and launcher code on other platforms. Alexey Semenyuk has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8254702: jpackage app launcher crashes on CentOS - Changes: - all: https://git.openjdk.java.net/jdk/pull/2320/files - new: https://git.openjdk.java.net/jdk/pull/2320/files/14d277a2..b493bcfd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320 PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS
On Fri, 29 Jan 2021 22:00:32 GMT, Phil Race wrote: >> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 >> >> The fix splits Linux app launcher in app launcher and launcher shared lib. >> App launcher is pure C and doesn't have C++ code. App launcher lib >> incorporates bulk of C++ code from app launcher. >> At startup app launcher loads launcher shared lib and calls functions it >> provides to get data for launching JVM (path to jli lib and arguments for >> JLI_Launch function call). >> App launcher unloads launcher shared lib before launching JVM to remove C++ >> runtime from the process memory. >> >> Getting rid of C++ code from app launcher required to rewrite app >> installation location lookup code from C++ to C. LinuxPackage.c source is C >> alternative for >> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp >> and >> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. >> >> Layout of jpackage's native code changed: >> - `common`: code shared between all jpackage binaries. >> - `applauncher`: launcher only code. >> - `applauncherlib`: launcher lib code on Linux and launcher code on other >> platforms. >> - `applaunchercommon`: code shared between launcher and launcher lib on >> Linux and launcher code on other platforms. > > So after this change if you bundle and run an app on Linux and then do "ps" > .. what is shown to be running ? Java or the app-name you expected ? "ps" will show app-name. It was never Java for jpackage apps before this patch and this patch doesn't change it. - PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS
On Fri, 29 Jan 2021 21:33:40 GMT, Alexey Semenyuk wrote: > Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 > > The fix splits Linux app launcher in app launcher and launcher shared lib. > App launcher is pure C and doesn't have C++ code. App launcher lib > incorporates bulk of C++ code from app launcher. > At startup app launcher loads launcher shared lib and calls functions it > provides to get data for launching JVM (path to jli lib and arguments for > JLI_Launch function call). > App launcher unloads launcher shared lib before launching JVM to remove C++ > runtime from the process memory. > > Getting rid of C++ code from app launcher required to rewrite app > installation location lookup code from C++ to C. LinuxPackage.c source is C > alternative for > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp > and > https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. > > Layout of jpackage's native code changed: > - `common`: code shared between all jpackage binaries. > - `applauncher`: launcher only code. > - `applauncherlib`: launcher lib code on Linux and launcher code on other > platforms. > - `applaunchercommon`: code shared between launcher and launcher lib on Linux > and launcher code on other platforms. So after this change if you bundle and run an app on Linux and then do "ps" .. what is shown to be running ? Java or the app-name you expected ? - PR: https://git.openjdk.java.net/jdk/pull/2320
RFR: 8254702: jpackage app launcher crashes on CentOS
Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 The fix splits Linux app launcher in app launcher and launcher shared lib. App launcher is pure C and doesn't have C++ code. App launcher lib incorporates bulk of C++ code from app launcher. At startup app launcher loads launcher shared lib and calls functions it provides to get data for launching JVM (path to jli lib and arguments for JLI_Launch function call). App launcher unloads launcher shared lib before launching JVM to remove C++ runtime from the process memory. Getting rid of C++ code from app launcher required to rewrite app installation location lookup code from C++ to C. LinuxPackage.c source is C alternative for https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp and https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp. Layout of jpackage's native code changed: - `common`: code shared between all jpackage binaries. - `applauncher`: launcher only code. - `applauncherlib`: launcher lib code on Linux and launcher code on other platforms. - `applaunchercommon`: code shared between launcher and launcher lib on Linux and launcher code on other platforms. - Commit messages: - 8254702: jpackage app launcher crashes on CentOS - 8254702: jpackage app launcher crashes on CentOS - 8254702: jpackage app launcher crashes on CentOS - 8254702: jpackage app launcher crashes on CentOS - 8254702: jpackage app launcher crashes on CentOS - 8254702: jpackage app launcher crashes on CentOS - It works! - 8254702: jpackage app launcher crashes on CentOS - strerror bugfix - Add helper makefile to build jpackage native code only - ... and 1 more: https://git.openjdk.java.net/jdk/compare/20e7df50...14d277a2 Changes: https://git.openjdk.java.net/jdk/pull/2320/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8254702 Stats: 1823 lines in 24 files changed: 1376 ins; 420 del; 27 mod Patch: https://git.openjdk.java.net/jdk/pull/2320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320 PR: https://git.openjdk.java.net/jdk/pull/2320
Re: RFR: 8254702: jpackage app launcher crashes on CentOS
On Fri, 29 Jan 2021 21:33:40 GMT, Alexey Semenyuk wrote:
> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702
>
> The fix splits Linux app launcher in app launcher and launcher shared lib.
> App launcher is pure C and doesn't have C++ code. App launcher lib
> incorporates bulk of C++ code from app launcher.
> At startup app launcher loads launcher shared lib and calls functions it
> provides to get data for launching JVM (path to jli lib and arguments for
> JLI_Launch function call).
> App launcher unloads launcher shared lib before launching JVM to remove C++
> runtime from the process memory.
>
> Getting rid of C++ code from app launcher required to rewrite app
> installation location lookup code from C++ to C. LinuxPackage.c source is C
> alternative for
> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp
> and
> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp.
>
> Layout of jpackage's native code changed:
> - `common`: code shared between all jpackage binaries.
> - `applauncher`: launcher only code.
> - `applauncherlib`: launcher lib code on Linux and launcher code on other
> platforms.
> - `applaunchercommon`: code shared between launcher and launcher lib on Linux
> and launcher code on other platforms.
make/modules/jdk.jpackage/applauncherlib.ld-version-script line 1:
> 1: {
Linker script is "just in case" to guarantee that only controlled set of
functions is exported from launcher lib. Current set of g++ OpenJDK command
line options disables export of functions with external linkage by default.
However in my local builds this was not the case and the whole C++ runtime
statically linked in launcher lib got exported. This prevented it from
unloading from launcher's process memory when it was `dlclose()`-ed. This
linker script used in linkage of launcher lib will guarantee this will not
happen.
src/jdk.jpackage/share/native/common/tstrings.cpp line 55:
> 53: ret = _vsntprintf_s(&*fmtout.begin(), fmtout.size(), _TRUNCATE,
> format, args);
> 54: #else
> 55: #if defined(__GNUC__) && __GNUC__ >= 5
Local build with older g++compiler bugfix.
-
PR: https://git.openjdk.java.net/jdk/pull/2320
