RFR: 8351491: Add info from release file to hserr file [v3]

[David Holmes]

On Thu, 3 Apr 2025 15:47:27 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> The release file of the JDK image contains useful info, for example the SOURCE used to built this image e.g.
>> SOURCE=".:git:21af8c7e7405"
>> Also the MODULES list is probably useful to have.
>> Add this info (or the complete content of the release file) to the hs_err files.
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   use one time PeriodicTask

A number of minor suggestions but this looks good to me. Thanks

src/hotspot/share/runtime/os.cpp line 1542:

> 1540: 
> 1541:     if (_image_release_file_content == nullptr) {
> 1542:       FILE *file = fopen(release_file, "rb");

Suggestion:

      FILE* file = fopen(release_file, "rb");

src/hotspot/share/runtime/os.cpp line 1543:

> 1541:     if (_image_release_file_content == nullptr) {
> 1542:       FILE *file = fopen(release_file, "rb");
> 1543:       if (!file) {

Suggestion:

      if (file != nullptr) {

Style: no implicit booleans

src/hotspot/share/runtime/os.cpp line 1563:

> 1561: 
> 1562:       size_t elements_read = fread(_image_release_file_content, 1, sz, file);
> 1563:       if (elements_read < (size_t)sz) _image_release_file_content[elements_read] = '\0';

Suggestion:

      if (elements_read < (size_t)sz) {
          _image_release_file_content[elements_read] = '\0';
      }

src/hotspot/share/runtime/os.cpp line 1564:

> 1562:       size_t elements_read = fread(_image_release_file_content, 1, sz, file);
> 1563:       if (elements_read < (size_t)sz) _image_release_file_content[elements_read] = '\0';
> 1564:       _image_release_file_content[sz] = '\0';

Shouldn't this be in an else?

src/hotspot/share/runtime/os.cpp line 1566:

> 1564:       _image_release_file_content[sz] = '\0';
> 1565:       // issues with \r in line endings on Windows, so better replace those
> 1566:       for (size_t i=0; i < elements_read; i++) {

Suggestion:

      for (size_t i = 0; i < elements_read; i++) {

src/hotspot/share/runtime/os.cpp line 1567:

> 1565:       // issues with \r in line endings on Windows, so better replace those
> 1566:       for (size_t i=0; i < elements_read; i++) {
> 1567:         if (_image_release_file_content[i] == '\r') { _image_release_file_content[i] = ' '; }

Suggestion:

        if (_image_release_file_content[i] == '\r') {
            _image_release_file_content[i] = ' ';
        }

src/hotspot/share/runtime/threads.cpp line 428:

> 426: }
> 427: 
> 428: // One-shot PeriodicTask subclass for reading release file

Suggestion:

// One-shot PeriodicTask subclass for reading the release file.
// The "period" of 100 is just an arbitrary initial delay.

src/hotspot/share/runtime/threads.cpp line 431:

> 429: class ReadReleaseFileTask : public PeriodicTask {
> 430:  public:
> 431:   ReadReleaseFileTask(size_t interval_time) : PeriodicTask(interval_time) {}

Suggestion:

  ReadReleaseFileTask() : PeriodicTask(100) {}

The "delay" can just be hard-wired here.

src/hotspot/share/runtime/threads.cpp line 436:

> 434:     os::read_image_release_file();
> 435: 
> 436:     // Reclaim our storage and disenroll ourself

Suggestion:

    // Reclaim our storage and disenroll ourself.

src/hotspot/share/runtime/threads.cpp line 596:

> 594:   }
> 595: 
> 596:   ReadReleaseFileTask* read_task = new ReadReleaseFileTask(100);

Suggestion:

  // Have the WatcherThread read the release file in the background.
  ReadReleaseFileTask* read_task = new ReadReleaseFileTask();

src/hotspot/share/utilities/vmError.cpp line 1436:

> 1434:   st->cr();
> 1435: 
> 1436:   // printing release file content

Suggestion:

  //  STEP("printing release file content")

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

PR Review: https://git.openjdk.org/jdk/pull/24244#pullrequestreview-2741592534
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027969664
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027969939
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027970357
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027970644
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027970966
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027971281
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027973768
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027973211
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027973903
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027974257
PR Review Comment: https://git.openjdk.org/jdk/pull/24244#discussion_r2027975466