On Thu, 12 Sep 2024 23:11:31 GMT, Justin Lu <j...@openjdk.org> wrote:
>> Please review this PR which restores the correct exception message when the
>> current working directory can not be found during java startup in
>> `initPhase1`.
>>
>> Both MacOS and Linux are expected to fail with `java.lang.Error: Properties
>> init: Could not determine current working directory` if the _user.dir_
>> system property cannot be initialized. Currently, MacOS now fails with
>> `java.lang.InternalError: platform encoding not initialized` and Linux fails
>> with `java.lang.InternalError: null property: user.dir` which are both
>> unexpected messages.
>>
>> In `System.c`,
>> `Java_jdk_internal_util_SystemProps_00024Raw_platformProperties` calls
>> `GetJavaProperties(JNIEnv *env)` which throws an internal error (with an
>> appropriate message) for Unix platforms when the current working directory
>> cannot be found. However, this exception is never checked and thus
>> unexpected failures occur later. NULL should be returned after the exception
>> is thrown, so that the initialization fails with the expected error when the
>> return value is checked.
>>
>>
>> // Get the platform specific values
>> sprops = GetJavaProperties(env);
>> CHECK_NULL_RETURN(sprops, NULL);
>>
>>
>> Testing done locally on both platforms.
>
> Justin Lu has updated the pull request incrementally with one additional
> commit since the last revision:
>
> return null in exception site instead
> Curiously in the Windows implementation of `GetJavaProperties(...)` in
> `java_props_md.c` in case of a failure to get the current working directory,
> we seem to just skip setting the working directory and don't raise any error:
>
> ```c
> /* Current directory */
> {
> WCHAR buf[MAX_PATH];
> if (GetCurrentDirectoryW(sizeof(buf)/sizeof(WCHAR), buf) != 0)
> sprops.user_dir = _wcsdup(buf);
> }
> ```
Hi Jai,
I noticed that too. I tried to replicate this scenario on Windows and was
always prevented from deleting the directory that I was currently in: _The
process cannot access the file because it is being used by another process._
But I'm sure there are ways to get the working directory deleted that I am
unaware of. To make this fix more complete, I can add similar behavior where an
exception is thrown for Windows if `GetCurrentDirectoryW` fails. AFAICT this
would be correcting a faulty error message with a proper one, and so there is
no behavioral change. What do you think?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20975#issuecomment-2349354516
