RFR: 8303942: os::write should write completely [v3]

Ioi Lam iklam at openjdk.org
Sat May 6 03:25:17 UTC 2023


On Fri, 5 May 2023 08:59:41 GMT, Afshin Zafari <duke at openjdk.org> wrote:

>> `os::write` is implemented using loops until the whole bytes are written. All uses of `os::write` in a loop are changed to single call. 
>> Platform dependent versions of the `os::write` are also renamed and moved to private sections accordingly.
>> Wrong uses/interpretations of return values from `os::write` in JFR code are corrected.
>> 
>> ###Test
>> local: hotspot tier1
>> mach5: tiers 1-5
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8303942: os::write should write completely

Looks good Just some minor nits.

src/hotspot/share/cds/filemap.cpp line 364:

> 362: 
> 363: void SharedClassPathEntry::copy_from(SharedClassPathEntry* ent, ClassLoaderData* loader_data, TRAPS) {
> 364:   assert(ent != NULL, "sanity");

This removal seems to be unrelated to this PR.

src/hotspot/share/jfr/recorder/repository/jfrEmergencyDump.cpp line 375:

> 373:     current_fd = open_exclusivly(fqn);
> 374:     if (current_fd != invalid_fd) {
> 375:       const size_t size = (size_t)file_size(current_fd);

There's an existing bug here: error code of -1 from `file_size` is not handled.

src/hotspot/share/jfr/writers/jfrStreamWriterHost.inline.hpp line 82:

> 80:     JfrJavaSupport::abort("Failed to write to jfr stream because no space left on device", false);
> 81:   }
> 82:   guarantee(num_written == 0, "Not all the bytes got written, or os::write() failed");

guarantee() seems to be a bad way of handling this. I would suggest filing an RFE for more robust error handling.

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

Marked as reviewed by iklam (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13750#pullrequestreview-1415642994
PR Review Comment: https://git.openjdk.org/jdk/pull/13750#discussion_r1186608738
PR Review Comment: https://git.openjdk.org/jdk/pull/13750#discussion_r1186608694
PR Review Comment: https://git.openjdk.org/jdk/pull/13750#discussion_r1186609298


More information about the hotspot-dev mailing list