Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v6]

2021-02-05 Thread Andy Herrick
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]

2021-02-05 Thread Erik Joelsson
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 [v6]

2021-02-05 Thread Alexey Semenyuk
> 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=2320=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2320=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