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