RFR: 8366272: The os::xxx APIs do not manage errno correctly

[David Holmes]

On Fri, 31 Oct 2025 13:44:09 GMT, Anton Artemov <aartemov at openjdk.org> wrote:

> Hi, please consider the following changes:
> 
> In this PR the handling of `errno` is improved by inspection of the codebase and fixing potential overwrites of `errno` by other methods. Potential overwrites are addressed using `ErrnoPreserver` functionality where necessary. 
> 
> Tested in tiers 1 - 5.

Sorry it has taken so long to look at this one.

I was surprised to see the limited cases covered in the PR, but looking at the code now I see that the motivating example in JBS:

     if (::FindClose(f) == 0) {
      errno = ::GetLastError();
      log_debug(os)("is_symbolic_link() failed to FindClose: GetLastError->%ld.", errno);
    } 


is no longer applicable after we untangled Windows use of `GetLastError` from use of `errno`. And it was not as prevalent as initially thought. I think we can dispense with `ErrorPreserver` in most cases.

Thanks

src/hotspot/os/aix/os_aix.cpp line 2334:

> 2332:         ErrnoPreserver ep;
> 2333:         ::close(fd);
> 2334:         errno = ep.saved_errno();

I suspect the intent is that we set EISDIR assuming the close succeeds, but if the close fails then errno reflects that error. Otherwise the trivial change here would be to reorder the two statements.

src/hotspot/os/windows/os_windows.cpp line 4785:

> 4783:       ErrnoPreserver ep;
> 4784:       os::free(wide_path);
> 4785:       errno = ep.saved_errno();

Just swap the statements

src/hotspot/os/windows/os_windows.cpp line 4805:

> 4803:     os::free(wide_path);
> 4804:     os::free(path_to_target);
> 4805:     errno = ep.saved_errno();

I would just move the free calls higher, then set errno after that.

src/hotspot/os/windows/os_windows.cpp line 5007:

> 5005:       ErrnoPreserver ep;
> 5006:       os::free(wide_path);
> 5007:       errno = ep.saved_errno();

Again in this simple case we can just switch the order and set errno after the free.

-------------

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28086#pullrequestreview-3550254075
PR Review Comment: https://git.openjdk.org/jdk/pull/28086#discussion_r2597146168
PR Review Comment: https://git.openjdk.org/jdk/pull/28086#discussion_r2597181639
PR Review Comment: https://git.openjdk.org/jdk/pull/28086#discussion_r2597178368
PR Review Comment: https://git.openjdk.org/jdk/pull/28086#discussion_r2597157272