RFR: 8284632: runtime/Thread/StopAtExit.java possibly leaking memory again

[Patricio Chilano Mateo]

On Mon, 25 Apr 2022 22:12:38 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

> Throwing async exceptions at exiting JavaThreads can leak the exception:
> 
> 1) HandshakeOperation::do_handshake() recognizes that the target thread
>     might be terminated, but needs to do some cleanup when the operation
>     is one that installs an async exception.
> 
> 2) JavaThread::exit() uses a NoAsyncExceptionDeliveryMark to protect the
>     VM's call out to Thread::exit() from async exceptions, but it needs to do
>     some cleanup when those async exceptions cannot be delivered because
>     the target thread has logically exited.
> 
> Also tweaked runtime/Thread/StopAtExit.java to alternate between throwing
> RuntimeException and ThreadDeath. The async exception queuing code accepts
> any exception, but it recognizes ThreadDeath as special. When a target thread
> already has a queued async ThreadDeath exception, then another exception will
> not be queued. So the test now throws RuntimeException on odd iterations and
> ThreadDeath on even iterations and the target thread will have at most two async
> exceptions queued up.
> 
> This fix has been tested with Mach5 Tier[1-7] (about to start a Tier8) and I also ran
> my StressWrapper_StopAtExit.java test using {release, fastdebug, and slowdebug}
> bits for 12 hours per config. No leaks detected. Previously, release bits would
> fail with OutOfMemoryException in ~2 minutes with StressWrapper_StopAtExit.java.

> Yes, good find!
> 
> When combining the code @dcubed-ojdk, @pchilano comments and my comments in my head I was unsure if the result was doable :) So I had to write the code to make sure I was not complete wrong.
> 
> Here is a change, diff against master: https://github.com/openjdk/jdk/compare/master...robehn:8284161?expand=1 Diff against this PR: https://github.com/dcubed-ojdk/jdk/compare/JDK-8284632...robehn:8284161?expand=1 Commit: [robehn at 0224ede](https://github.com/robehn/jdk/commit/0224ede15980006ce547de72ee0a778b10054b45)
> 
> Thanks
>
@dcubed-ojdk I now see the other issue you were trying to fix which is never installing the async handshake in the first place because the thread already terminated(so the queue will actually be empty), but we already created the AsyncExceptionHandshake. The is_existing() check was meant to solve the issue of adding an operation between the _thread_exiting and _thread_terminated interval. Your check addressed the leak after _thread_terminated was set. @robehn your patch looks good and fixes both issues.

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

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