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

[Thomas Stuefe]

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.

Okay, that makes sense considering that people tend to copy paste solutions. 

I leave this up to Simon, then (also the question whether he wants to tackle rewriting Windows thread startup in a manner that we prevent this situation altogether). After all, the test now is problemlisted, so the urgency is gone.

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

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