Hi Aleksei,

Looks good!

- Alexey

On 7/21/2020 11:42 AM, Aleksei Voitylov wrote:
Hi,

This is the updated fix which checks if LD_LIBRARY_PATH has been changed
during jpackage executions:

http://cr.openjdk.java.net/~avoitylov/webrev.8248239.03/

The fix stores hash from LD_LIBRARY_PATH in _JPACKAGE_LAUNCHER env
variable. Until C++14 becomes mandatory for OpenJDK, a custom hash
algorithm is used because standard C++ hash requires -std=c++11 or
-std=gnu++11 compiler options.

All test/jdk/tools/jpackage tests but ModulePathTest3.java pass on Linux
x86_64, MacOSX x86_64, and Windows x86_64. The test ModulePathTest3.java
is excluded in test/jdk/ProblemList.txt (8248418 generic-all).

Thanks,

-Aleksei

On 26/06/2020 20:23, Alexey Semenyuk wrote:
Hi Aleksei,

I think the idea of partial reading data from cfg file when the
launcher is restarted has a flaw.
It would be better if app launcher can detect if it was restarted and
if it was, not read cfg file at all, but pass command line arguments
as is in JLI_Launch().
Let my think on ideas how to address this.

- Alexey

On 6/26/2020 7:16 AM, Aleksei Voitylov wrote:
Hi Alexey,

Thank you for looking into it. As far as using parent pid, it does not
seem to work as example [1] demonstrates. The parent process remains the
same after re-execution and does not become the current process.

I checked passing arguments in the "ArgOption" section using several
cases [2, 3, 4] with the proposed fix and app re-execution. The proposed
fix handles this case well, and the results are the same as without the
fix when the app is not re-executed.

Case [3] where only JavaOptions without ArgOptions are passed to app
looks suspicious because default ArgOptions are not used. But it works
the same way without the proposed fix, which seems like a different
issue. According to jpackage documentation:

    --arguments main class arguments
          Command line arguments to pass to the main class if no command
line arguments are given to the launcher.

I filed a separate issue [5] to handle that.

Thanks,
-Aleksei


[1]
cd jdk-dev
make jdk-image
export PATH=build/linux-x86_64-server-release/images/jdk/bin:$PATH
export
LD_LIBRARY_PATH=build/linux-x86_64-server-release/images/jdk/lib/server
jpackage --dest output --name app --type app-image --module-path
input-modules --module com.hello/com.hello.Hello --arguments "A A2 A"
--verbose --java-options -Dparam1=Param1
./output/app/bin/app -Dparam2=Param2 B B2 B
-------------
pid: 16007, current process: /home/sample/jdk-dev/output/app/bin/app
pid: 15927, parent  process: /bin/bash
JvmLauncher args: 10 [/home/sample/jdk-dev/output/app/bin/app
-Dparam1=Param1 --module-path
/home/sample/jdk-dev/output/app/lib/app/mods -m
com.hello/com.hello.Hello -Dparam2=Param2 B B2 B ]
pid: 16007, current process: /home/sample/jdk-dev/output/app/bin/app
pid: 15927, parent  process: /bin/bash
JvmLauncher args: 15 [/home/sample/jdk-dev/output/app/bin/app
-Dparam1=Param1 --module-path
/home/sample/jdk-dev/output/app/lib/app/mods -m
com.hello/com.hello.Hello -Dparam1=Param1 --module-path
/home/sample/jdk-dev/output/app/lib/app/mods -m
com.hello/com.hello.Hello -Dparam2=Param2 B B2 B ]
-------------

[2]
jpackage --dest output --name app --type app-image --module-path
input-modules --module com.hello/com.hello.Hello --arguments "A A2 A"
--java-options -Dparam1=Param1
./output/app/bin/app
JvmLauncher args: 9 [output/app/bin/app -Dparam1=Param1 --module-path
output/app/lib/app/mods -m com.hello/com.hello.Hello A A2 A ]
JvmLauncher args: 9 [output/app/bin/app -Dparam1=Param1 --module-path
output/app/lib/app/mods -m com.hello/com.hello.Hello A A2 A ]

[3]
jpackage --dest output --name app --type app-image --module-path
input-modules --module com.hello/com.hello.Hello --arguments "A A2 A"
--java-options -Dparam1=Param1
./output/app/bin/app -Dparam2=Param2
JvmLauncher args: 7 [output/app/bin/app -Dparam1=Param1 --module-path
output/app/lib/app/mods -m com.hello/com.hello.Hello -Dparam2=Param2 ]
JvmLauncher args: 7 [output/app/bin/app -Dparam1=Param1 --module-path
output/app/lib/app/mods -m com.hello/com.hello.Hello -Dparam2=Param2 ]

[4]
jpackage --dest output --name app --type app-image --module-path
input-modules --module com.hello/com.hello.Hello --arguments "A A2 A"
--java-options -Dparam1=Param1
./output/app/bin/app -Dparam2=Param2 B B2 B
JvmLauncher args: 10 [output/app/bin/app -Dparam1=Param1 --module-path
output/app/lib/app/mods -m com.hello/com.hello.Hello -Dparam2=Param2 B
B2 B ]
JvmLauncher args: 10 [output/app/bin/app -Dparam1=Param1 --module-path
output/app/lib/app/mods -m com.hello/com.hello.Hello -Dparam2=Param2 B
B2 B ]

[5] https://bugs.openjdk.java.net/browse/JDK-8248397


On 24/06/2020 19:34, Alexey Semenyuk wrote:
Aleksei,

If I get it right, the fix would not work for the case when there are
`arguments` properties in `ArgOptions` section of a config file.
Why not just check if the parent process is the same executable as the
current one and if this is the case don't read data from the config
file but pass executable arguments as is in JLI_Launch() call?

- Alexey

On 6/24/2020 11:48 AM, Aleksei Voitylov wrote:
Hi,

There are systems that update LD_LIBRARY_PATH or directly return
JNI_TRUE in method RequiresSetenv(const char *jvmpath) from java_md.c
file to request re-execution of the current executable. This leads to
the fact that jpackage application adds some provided arguments twice.

Bug: https://bugs.openjdk.java.net/browse/JDK-8248239
Fix: http://cr.openjdk.java.net/~avoitylov/webrev.8248239.00/

The root cause of the issue is that jpackage application expects one
number of arguments but JLI reexecutes them with another number of
arguments.
    For example, a jpackage application can be run with arguments:
       ./app/bin/app -Dparam2=Param2 B1 B2 B3
it adds arguments from the config file and calls JLI with arguments:
       app/bin/app -classpath  -Dparam1=Param1 -m
com.hello/com.hello.Hello
-Dparam2=Param2 B1 B2 B3
JLI re-executes the app with new arguments so the app adds some
arguments one more time after the re-execution.
       ./app/bin/app -classpath  -Dparam1=Param1 -m
com.hello/com.hello.Hello -classpath  -Dparam1=Param1 -m
com.hello/com.hello.Hello -Dparam2=Param2 B1 B2 B3

A step by step example that illustrates the issue:

Run jpackage to create an executable application:
     jpackage --dest output --name app --type app-image --module-path
input-modules --module com.hello/com.hello.Hello --arguments "A1 A2
A3"
--verbose --java-options -Dparam1=Param1

Executable application with the app/lib/app/app.cfg config file is
created:
---- app.cfg  ----
[Application]
app.name=app
app.version=1.0
app.runtime=$ROOTDIR/lib/runtime
app.identifier=com.hello
app.classpath=
app.mainmodule=com.hello/com.hello.Hello

[JavaOptions]
java-options=-Dparam1=Param1

[ArgOptions]
arguments=A1
arguments=A2
arguments=A3
-----------

Run the application:
      ./output/app/bin/app -Dparam2=Param2 B1 B2 B3

Chain of JDK methods execution:

LinuxLauncher main()
      args: 5 [./app/bin/app -Dparam2=Param2 B1 B2 B3 ]
AppLauncher createJvmLauncher()
      args: 4 [-Dparam2=Param2 B1 B2 B3 ]
JvmLauncher.cpp Jvm::initFromConfigFile() - adds JavaOptions from
app.cfg
      args: 10 [app/bin/app -classpath  -Dparam1=Param1 -m
com.hello/com.hello.Hello -Dparam2=Param2 B1 B2 B3 ]
AppLauncher Jvm::launch() -  JLI re-executes the app
LinuxLauncher main()
     args: 10 [app/bin/app -classpath  -Dparam1=Param1 -m
com.hello/com.hello.Hello -Dparam2=Param2 B1 B2 B3 ]
AppLauncher createJvmLauncher()
      args: 9 [-classpath  -Dparam1=Param1 -m com.hello/com.hello.Hello
-Dparam2=Param2 B1 B2 B3 ]
JvmLauncher.cpp Jvm::initFromConfigFile() - adds JavaOptions from
app.cfg
      args: 15 [./app/bin/app -classpath  -Dparam1=Param1 -m
com.hello/com.hello.Hello -classpath  -Dparam1=Param1 -m
com.hello/com.hello.Hello -Dparam2=Param2 B1 B2 B3 ]
       ^^^

Some arguments like "-classpath  -Dparam1=Param1 -m
com.hello/com.hello.Hello" are added twice.

Tested with test/jdk/tools/jpackage/share/jdk/jpackage with no
regressions on Linux, Windows, Mac. On systems affected, the tests
now pass.

Thanks,

-Aleksei



Reply via email to