RFR: 8290043: serviceability/attach/ConcAttachTest.java failed "guarantee(!CheckJNICalls) failed: Attached JNI thread exited without being detached" [v3]

[David Holmes]

On Thu, 9 Jan 2025 22:00:39 GMT, David Holmes <dholmes at openjdk.org> wrote:

>>> smr_delete is deliberately not put into exit so that it is guaranteed noone ever mistakenly puts new exit logic after the delete occurs.
>> 
>> You can always put a comment in thread->exit() that smr_delete should be last.
>> 
>> The issue was that the asymmetry caused an NMT call to be missed in one of the exit paths, and you partially fixed this by moving the NMT call to exit() but it will be really easy to break this again by adding some new shutdown call at post_run() before smr_delete() assuming that's a good place to put the call (also because the non-thread call is there).  exit() is a really long function and does a kitchensink of things.  Is smr_delete() called after exit() every time?  It seems so.
>> 
>> I'd prefer the NMT call be inlined in the post_run() duplicated actions in the JNI detach code.  If you're going to duplicate the smr_delete() call.
>
> After discussing this with Coleen I've discovered that [JDK-8252921](https://bugs.openjdk.org/browse/JDK-8252921) introduced additional breakage because we no longer unregister in the case of a failed attach. This is all caused by moving the call from the destructor so I will look at putting it back for JavaThreads.

Using the destructor is wrong - see updated PR description.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22924#discussion_r1909678743