On Thu, 21 May 2026 22:53:09 GMT, Ashay Rane <[email protected]> wrote:
>> Although the original implementation of `JLI_Open()` borrowed from the >> code in src/hotspot/os/windows/os_windows.cpp, the improvements to >> os_windows.cpp don't seem to have been applied to java_md.c, causing >> some tests to fail when the path to JAR files is longer than `MAXPATH` >> (i.e. 260) characters on Windows (see associated JBS issue 8385024 for >> details). Since `JLI_Open()` is not just invoked inside tests, this is >> not a test-specific issue, so fixing the test is not the right solution. >> >> This patch applies the recent changes from os_windows.cpp to java_md.c >> so that `JLI_Open()` can correctly handle longer than `MAXPATH` paths. >> The new code is almost the same as that in `wide_abs_unc_path()` in >> os_windows.cpp, except that the code in java_md.c uses `JLI_MemAlloc()` >> and `JLI_MemFree()` for memory allocation and deallocation. >> >> Although it would be ideal to have just one implementation between >> HotSpot and the launcher, the dependencies of the two components >> prevents us from having a single implementation. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Ashay Rane has updated the pull request incrementally with one additional > commit since the last revision: > > Address PR comments > > 1. Optimize for the common case when the JAR path is shorter than > `MAX_PATH` by using a stack buffer of size length `MAX_PATH`. If the > full path is longer than that, then we allocate a heap buffer of the > desired size before calling `GetFullPathNameW()` with a buffer of the > right size. > > 2. Normalize the path before called `set_path_prefix()`, just like in > os_windows.cpp. This ensures that prefixed paths with forward > slashes work correctly. > > 3. Renamed `create_unc_path()` to `convert_to_absolute_path()` to > reflect the common action performed by the function. > > 4. Dropped the accidental overwritting of `errno` after calling > `_wopen()`. Thanks Alan! You were right in your hunch that `\?`-prefixed paths with forward slashes were not being handled correctly. I had missed copying an additional function (`os::native_path()`) from os_windows.cpp. == before == C:\>C:\msys64\home\JEG\openjdk-jdk\build\windows-x86_64-server-fastdebug\images\jdk\bin\java -jar C:\msys64\home\JEG\openjdk-jdk\x\hello.jar hi C:\>C:\msys64\home\JEG\openjdk-jdk\build\windows-x86_64-server-fastdebug\images\jdk\bin\java -jar C:/msys64/home/JEG/openjdk-jdk/x/hello.jar hi C:\>C:\msys64\home\JEG\openjdk-jdk\build\windows-x86_64-server-fastdebug\images\jdk\bin\java -jar \?\C:\msys64\home\JEG\openjdk-jdk\x\hello.jar hi C:\>C:\msys64\home\JEG\openjdk-jdk\build\windows-x86_64-server-fastdebug\images\jdk\bin\java -jar \?\C:/msys64/home/JEG/openjdk-jdk/x/hello.jar Error: Unable to access jarfile \?\C:/msys64/home/JEG/openjdk-jdk/x/hello.jar == after == C:\>C:\msys64\home\JEG\openjdk-jdk\build\windows-x86_64-server-fastdebug\images\jdk\bin\java -jar C:\msys64\home\JEG\openjdk-jdk\x\hello.jar hi C:\>C:\msys64\home\JEG\openjdk-jdk\build\windows-x86_64-server-fastdebug\images\jdk\bin\java -jar C:/msys64/home/JEG/openjdk-jdk/x/hello.jar hi C:\>C:\msys64\home\JEG\openjdk-jdk\build\windows-x86_64-server-fastdebug\images\jdk\bin\java -jar \?\C:\msys64\home\JEG\openjdk-jdk\x\hello.jar hi C:\>C:\msys64\home\JEG\openjdk-jdk\build\windows-x86_64-server-fastdebug\images\jdk\bin\java -jar \?\C:/msys64/home/JEG/openjdk-jdk/x/hello.jar hi In trying to avoid having to call `GetFullPathNameW()` twice for the common case when file paths are shorter than 260 characters, I am using a buffer of size `MAX_PATH` on stack and only when the full path is found to be longer than that do I allocate a heap buffer of the size told to us by `GetFullPathNameW()`. The fact that the returned buffer can be either on the stack or the heap introduces a bit of complexity, requiring a boolean flag (`needs_free`). Let me know if you think there's a more elegant implementation. I also tried adding a test to test/jdk/tools/launcher/Arrrghs.java (mimicking `testLongPathJarFile()` but so that the working directory has a long path but the value passed to the "-jar" flag itself is short): @Test void testLongPathJarFileRelative() throws IOException { if (!isWindows) { return; } String longPathPart = "longpathtest_longpathtest/"; String longPathStr = longPathPart.repeat(15); Path longPath = Paths.get(longPathStr); Path jarPath = Files.createDirectories(longPath).resolve("elp.jar"); File elp = jarPath.toFile(); createJar(elp, new File("Foo"), "public static void main(String[] args){ System.out.println("Hello from ELP"); }"); // Bypassing `doExec()` since we need to set the current working directory. String absDir = longPath.toAbsolutePath().toString(); ProcessBuilder pb = new ProcessBuilder(javaCmd, "-jar", "elp.jar"); pb.directory(new File("\\\?\" + absDir)); pb.redirectErrorStream(true); try { Process p = pb.start(); List<String> output = new ArrayList<>(); try (BufferedReader rdr = new BufferedReader( new InputStreamReader(p.getInputStream()))) { String line; while ((line = rdr.readLine()) != null) { output.add(line); } } p.waitFor(); if (p.exitValue() != 0) { throw new RuntimeException("Process exited with " + p.exitValue() + "\nOutput: " + output); } if (output.stream().noneMatch(s -> s.contains("Hello from ELP"))) { throw new RuntimeException("Expected output not found: " + output); } } catch (InterruptedException e) { throw new RuntimeException(e); } } However, this fails during execution since `ProcessBuilder` seems to strip away the `\?` prefix, causing the following exception: Caused by: java.io.IOException: Cannot run program "C:\msys64\home\JEG\openjdk-jdk\build\windows-x86_64-server-fastdebug\images\jdk\bin\java.exe" (in directory "C:\msys64\home\JEG\openjdk-jdk\build\windows-x86_64-server-fastdebug\test-support\jtreg_test_jdk_tools_launcher_Arrrghs_java\scratch\0\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest\longpathtest_longpathtest"): CreateProcess error=267, The directory name is invalid at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1118) at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1052) at Arrrghs.testLongPathJarFileRelative(Arrrghs.java:527) ... I need to trace the call from `ProcessBuilder.start()` to [`processCreate()`](https://github.com/openjdk/jdk/blob/master/src/java.base/windows/native/libjava/ProcessImpl_md.c#L340) in ProcessImpl_md.c to see where the prefix is getting stripped. Let me know if you think we should fix that first before proceeding further on this patch. ------------- PR Comment: https://git.openjdk.org/jdk/pull/31209#issuecomment-4513555642
