RFR: 8340401: DcmdMBeanPermissionsTest.java and SystemDumpMapTest.java fail with assert(_stack_base != nullptr) failed: Sanity check

[David Holmes]

On Sun, 22 Sep 2024 09:20:52 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> I don't think this is limited to JavaThread
>> 
>> @tstuefe whether other kinds of thread iterators have potential issues with the state of the threads they may observe is not relevant to the question of whether the proposed check goes in the iteration loop or the HANDLE_THREAD macro - we don't process any GC threads with HANDLE_THREAD. The JavaThread iterator can return a thread that has not yet set its stack so the code using the iterator needs to deal with that if it might be an issue.
>
>> > I don't think this is limited to JavaThread
>> 
>> @tstuefe whether other kinds of thread iterators have potential issues with the state of the threads they may observe is not relevant to the question of whether the proposed check goes in the iteration loop or the HANDLE_THREAD macro - we don't process any GC threads with HANDLE_THREAD. The JavaThread iterator can return a thread that has not yet set its stack so the code using the iterator needs to deal with that if it might be an issue.
> 
> We iterate GC threads (https://github.com/openjdk/jdk/blob/9b54225e35da5411e7a9e51320adc3b014b92261/src/hotspot/share/nmt/memMapPrinter.cpp#L226) here; it calls vma_touches_thread_stack, which is why I proposed in my last comment (https://github.com/openjdk/jdk/pull/21091#pullrequestreview-2317615786) to fix that function. Am I overlooking something?

@tstuefe I think this is a thread lifecycle management issue and so anyone using the JavaThread iterator or GCworkersDo needs to be aware of the possibility of a thread having a null stackbase. Having the check as close as possible to the iteration loop (whichever kind) makes this more visible, rather than hiding it inside another function. I would rather `vma_touches_thread_stack` be guaranteed to get "good threads" than writing it to allow for "bad ones". Of course even better if we simply don't allow this kind of thread "badness" in the first place.

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

PR Comment: https://git.openjdk.org/jdk/pull/21091#issuecomment-2367188837