On Mon, 29 Jun 2026 20:44:06 GMT, David Holmes <[email protected]> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add warning for non-writable /tmp directory suggesting to use AltTempDir.
>
> src/hotspot/share/runtime/arguments.cpp line 1725:
>
>> 1723:
>> 1724: os::check_temp_directory();
>> 1725: os::init_container_support();
>
> Can't we place `check_temp_directory()` call inside the Linux
> `pd_init_container_support()` function and so avoid needing to add yet
> another method to the OS API (especially when it is actually only needed on
> one OS).
I thought of that but it's not specifically for containers. It's presumably
for containers but you don't need containers to use AltTempDir.
> test/jdk/com/sun/tools/attach/JvmTempDirTest.java line 84:
>
>> 82: runNoExistTest(noExist);
>> 83:
>> 84: Path veryLongDir = Files.createTempDirectory(Path.of("/tmp"),
>> "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
>
> We need to test values around the UNIX_PATH_MAX-22 (or 20) length limit, not
> just one arbitrarily long one.
Will fix this test.
> test/jdk/com/sun/tools/attach/JvmTempDirTest.java line 229:
>
>> 227:
>> 228: private static void runRelativeTest(Path tmpDir) throws Throwable {
>> 229:
>
> Suggestion:
>
> private static void runRelativeTest(Path tmpDir) throws Throwable {
I don't see a change that you suggested, but I added them to the batch.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31407#discussion_r3498725721
PR Review Comment: https://git.openjdk.org/jdk/pull/31407#discussion_r3498703473
PR Review Comment: https://git.openjdk.org/jdk/pull/31407#discussion_r3498712442
