RFR: 8319897: Move StackWatermark handling out of LockStack::contains

[Stefan Karlsson]

On Mon, 13 Nov 2023 02:16:41 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> There are StackWatermark handling inside LockStack::contains. This is an indication that either:
>> 1) some thread has failed to start it's own StackWatermark processing, or
>> 2) that some thread is looking into the oops of another thread without starting the StackWatermark handling. The latter is usually more common.
>> 
>> I propose that we hoist the StackWatermark processing out of the LockStack into the code that actually performs pokes around in other thread's data. This is usually where we put the StackWatermark processing calls.
>> 
>> Note: that this doesn't fix a bug, it merely moves the processing code nearer the source of the problematic calls. I've found that jmm_GetThreadInfo looks straight into running thread's lock stacks without while those threads are running. That is a race that we should get rid of and I've created [JDK-8319899](https://bugs.openjdk.org/browse/JDK-8319899) for that issue.
>
> src/hotspot/share/runtime/lockStack.inline.hpp line 110:
> 
>> 108:   // Can't poke around in thread oops without having started stack watermark processing.
>> 109:   StackWatermark* watermark = StackWatermarkSet::get(get_thread(), StackWatermarkKind::gc);
>> 110:   assert(watermark == nullptr || watermark->processing_started(), "Processing must have started!");
> 
> Is line #109 only to support the assertion? If so it should be ifdef'd. Though for an assertion I would prefer to see some kind of `StackWatermark::is safe_for(Thread* t)` API.

Yes, it is only for the assertions. I prefer to avoid adding ifdefs because I find them highly distracting. I was considering adding a function, but couldn't immediately come up with a great name. I'll send out a new proposal with a new function. Thanks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16609#discussion_r1390750542