RFR: JDK-8255661: TestHeapDumpOnOutOfMemoryError fails with EOFException

[Richard Reingruber]

On Thu, 22 Apr 2021 14:16:21 GMT, Ralf Schmelter <rschmelter at openjdk.org> wrote:

> This fixes a race condition in the CompressionBackend class of the heap dump code.
> 
> The race happens when the thread iterating the heap wants to write the data it has collected. If the compression backend has worker threads, the buffer to write would just be added to a queue and the worker threads would then compress (if needed) and write the buffer. But if no worker threads are present, the thread doing the iteration must do this itself.
> 
> The iterating thread checks the _nr_of_threads member under lock protection and if it is 0, it assume it would have to do the work itself. It then releases the lock and enters the loop of the worker threads for one round. But after the lock has been released, a worker thread could be registered and handle the buffer itself. Then the iterating thread would wait until another buffer is available, which will never happen.
> 
> The fix is to take the buffer to write out of the queue in the iterating thread under lock protection and the do the unlocking.

Hi Ralf,

your change looks good to me.

Thanks for fixing,
Richard.

src/hotspot/share/services/heapDumperCompression.cpp line 262:

> 260: }
> 261: 
> 262: void CompressionBackend::thread_loop() {

You could simplify `CompressionBackend::thread_loop()` further:


void CompressionBackend::thread_loop() {
  {
    MonitorLocker ml(_lock, Mutex::_no_safepoint_check_flag);
    _nr_of_threads++;
  }

  WriteWork* work = get_work();
  while (work != NULL) {
      do_compress(work);
      finish_work(work);
      work = get_work();
  }

  MonitorLocker ml(_lock, Mutex::_no_safepoint_check_flag);
  _nr_of_threads--;
  assert(_nr_of_threads >= 0, "Too many threads finished");
  ml.notify_all();
}

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

Marked as reviewed by rrich (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3628