Hi Alexey,
Looks good. I do not see any issues with keeping code which alters PATH.
I think it might be better to keep it, just in case.
Thanks,
Alexander
On 5/12/2020 8:42 AM, Alexey Semenyuk wrote:
Changed the patch to try AddDllDirectory() first and alter PATH as the
last resort. Webrev at [1]
Log output:
---
$ env JPACKAGE_DEBUG=true ./JTwork/scratch/output/test/test.exe
[TRACE] applauncher.cpp:217: Entering AppLauncher::launch
[TRACE] applauncher.cpp:100: Launcher config file path:
"C:\Users\test\8244634\jdk-15_b22\JTwork\scratch\output\test\app\test.cfg"
[TRACE] winlauncher.cpp:70: Entering
`anonymous-namespace'::loadDllWithAddDllDirectory
[TRACE] winlauncher.cpp:86:
AddDllDirectory(C:\Users\test\8244634\jdk-15_b22\JTwork\scratch\output\test\runtime\bin):
OK
[TRACE] winlauncher.cpp:94:
LoadLibraryEx(C:\Users\test\8244634\jdk-15_b22\JTwork\scratch\output\test\runtime\bin\jli.dll,
LOAD_LIBRARY_SEARCH_DEFAULT_DIRS): 00007FFBED200000
[TRACE] winlauncher.cpp:0: Exiting
`anonymous-namespace'::loadDllWithAddDllDirectory (entered at
winlauncher.cpp:70)
[TRACE] jvmlauncher.cpp:177: JVM library:
"C:\Users\test\8244634\jdk-15_b22\JTwork\scratch\output\test\runtime\bin\jli.dll"
hello: Environment supports a display
jpackage test application
args.length: 0
-Dparam1=Some Param 1
-Dparam2=Some "Param" 2
-Dparam3=Some "Param" with " 3
hello: Output file: [appOutput.txt]
[TRACE] applauncher.cpp:0: Exiting AppLauncher::launch (entered at
applauncher.cpp:217)
---
AddDllDirectory/LoadLibraryEx works. However not with
LOAD_LIBRARY_SEARCH_USER_DIRS flag, but with wider
LOAD_LIBRARY_SEARCH_DEFAULT_DIRS.
Should we keep the code that would alter PATH in case
AddDllDirectory() doesn't work?
[1] http://cr.openjdk.java.net/~asemenyuk/8244634/webrev.01
- Alexey
On 5/12/2020 10:33 AM, Alexey Semenyuk wrote:
> Is the problem that by removing the copy of the microsoft dlls from
the applications bin directory, then on machines that do not have all
of these dll's already in the PATH (usually in C:\windows\system32)
they can no longer be found ?
Yes.
> Is there a way you can link the launcher (e.g., something similar
to RPATH on Unix systems) to look in the right place relative to the
launcher?
The problem with failure to load jli.dll from the app launcher is
that the system fails to load dependent dlls for jli.dll (MSVC
run-time libs) from the directory with jli.dll even though the full
path to jli.dll is specified in LoadLibrary() call. Launcher itself
is statically linked with MSVC run-time libs. It it not an issue.
> Did you try to load jli.dll by specifying full path to jli.dll when
calling LoadLibary?
Yes. Launcher loads jli.dll with the full path specified. The problem
why LoadLibrary() fails to load it from the full path is that
depending on OS/DLL loading settings LoadLibrary() would or would NOT
look for dependent dlls in the directory where jli.dll is located.
The tricky part about AddDllDirectory() is that it would work for
LoadLibraryEx() with LOAD_LIBRARY_SEARCH_USER_DIRS only. Is this how
Windows implicitly loads MSVC run-time libs before it completes
explicit request to load jli.dll from the app launcher? I don't know.
PATH env variable will be altered only in case the first attempt to
load jli.dll fails. Value of PATH can be restored after jli.dll is
loaded. This piece of code can be added to the patch. I'll also give
another try to AddDllDirectory() with LoadLibraryEx() call.
- Alexey
On 5/12/2020 8:31 AM, Kevin Rushforth wrote:
Is there a way you can link the launcher (e.g., something similar to
RPATH on Unix systems) to look in the right place relative to the
launcher? Otherwise, the solution with adding to the PATH seems OK
to me.
-- Kevin
On 5/12/2020 5:22 AM, Andy Herrick wrote:
Is the problem that by removing the copy of the microsoft dlls from
the applications bin directory, then on machines that do not have
all of these dll's already in the PATH (usually in
C:\windows\system32) they can no longer be found ?
I don't like manually manipulating the PATH env variable either,
but if so I don't see what else can be done short of putting the
app exe in the bin dir of the runtime.
/Andy
On 5/11/2020 9:37 PM, Alexander Matveev wrote:
Hi Alexey,
Updating PATH does not look like good solution to me. Did you try
to load jli.dll by specifying full path to jli.dll when calling
LoadLibary? If it does not work, then for AddDllDirectory() did
you used LoadLibrary() or LoadLibraryEx() with
LOAD_LIBRARY_SEARCH_USER_DIRS? According to doc you need to use
LoadLibraryEx() with flag when using AddDllDirectory().
Thanks,
Alexander
On 5/11/2020 4:36 PM, Alexey Semenyuk wrote:
Please review fix [2] for jpackage bug [1].
Fix failure to load jli.dll from app launcher. The fix is to
append path to directory with jli.dll to PATH env variable and
load jli.dll with altered value of PATH if the first attempt to
load jli.dll without altering PATH fails.
- Alexey
[1] https://bugs.openjdk.java.net/browse/JDK-8244634
[2] http://cr.openjdk.java.net/~asemenyuk/8244634/webrev.00