RFR: JDK-8244634:, LoadLibraryW failed from tools/jpackage tests after JDK-8242302

[Alexander Matveev]

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
>>>>>>
>>>>>
>>>
>>
>