On Mon, 29 Jun 2026 20:22:34 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/os/linux/os_linux.cpp line 1565:
>
>> 1563: // /proc/{vmid}/root/tmp/{PERFDATA_NAME_user}, otherwise
>> /tmp/{PERFDATA_NAME_user}, so we add 22.
>> 1564:
>> 1565: // We also check that it is a fully qualified pathname.
>
> Suggestion:
>
> // We need to check that any given alternate temporary directory name isn't
> too long,
> // is a writable directory, and specifies an absolute path. Revert back to
> hardcoded /tmp otherwise.
> // Since the attach mechanism uses the socket name length, this severely
> limits the length of the
> // alternate temporary directory name.
>
> // If in a containerized process, temp can be used to compose the dirname of
> // /proc/{vmid}/root/tmp/{PERFDATA_NAME_user}, otherwise
> /tmp/{PERFDATA_NAME_user}, so we allow
> // 22 characters for the prefix.
> const int CONTAINER_PREFIX_LEN = 22;
That said, given that we later do `"%s/.java_pid%d"` shouldn't the length check
also allow for the additional 20 characters for that? Otherwise we will fail
with no clear indicator why.
Actually I'm a bit confused about the possibilities. Isn't the container-prefix
only used for perfdata, not attach? And perfdata uses PATH_MAX anyway. So we
don't need to allow for the 22 in the general case, but we do need to allow
for the `/.java_pid%d` - which is 20.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31407#discussion_r3494540887
