RFR: 8340491: Thread stack-base assertion should report which thread has the un-set stack

[Stefan Karlsson]

On Fri, 20 Sep 2024 02:25:19 GMT, David Holmes <dholmes at openjdk.org> wrote:

> Please review this simple enhancement to an assertion so we can identify which thread had the problem.
> 
> I had to move the function to the cpp file due to include file issues.
> 
> We are limited in what we can print due to this being used very early in the thread initialization process - in particular no ResourceMarks are possible so we can't print the name.
> 
> Testing:
>  - tier 5 (used to debug [JDK-8340401](https://bugs.openjdk.org/browse/JDK-8340401))
>  - tiers 1-3 sanity
> 
> Thanks

Looks fine, but could you explain when this is needed?

If it is the current thread that is crashes then we already print the thread info.

If it is crashing when called on another thread, it might be easier to add informative asserts in that sub-system instead. You could for example add a non-asserting getter and perform all relevant checks on the caller side.

Another alternative could be to leverage the VMErrorCallback functionality I introduced a little while ago. With that you can register a callback to be called by the hs_err printer if the current thread crashes. With that you can have much more context about what the calling code was doing and what information that thread has available. That would look something like this (I've not compiled this, so there might be errors):


diff --git a/src/hotspot/share/nmt/memMapPrinter.cpp b/src/hotspot/share/nmt/memMapPrinter.cpp
index 5f920b102a9..bfceec19fca 100644
--- a/src/hotspot/share/nmt/memMapPrinter.cpp
+++ b/src/hotspot/share/nmt/memMapPrinter.cpp
@@ -203,11 +203,32 @@ static void print_thread_details(uintx thread_id, const char* name, outputStream
   st->print_raw(tmp);
 }

+class VMAInspectionErrorCallback : public VMErrorCallback {
+  const void* const _from;
+  const void* const _to;
+  Thread* const     _thread;
+
+public:
+  VMAInspectionErrorCallback(const void* from, const void* to, Thread* thread) :
+    _from(from), _to(to), _thread(thread) {}
+
+  void call(outputStream* st) override {
+    st->print_cr("Crashing while printing VMA details for " PTR_FORMAT " " PTR_FORMAT " for thread %s with id: %d",
+                 p2i(_from),
+                 p2i(_to),
+                 _thread->name(),
+                 _thread->osthread() != nullptr ? _thread->osthread()->thread_id() : 0);
+  }
+};
+
 // Given a region [from, to), if it intersects a known thread stack, print detail infos about that thread.
 static void print_thread_details_for_supposed_stack_address(const void* from, const void* to, outputStream* st) {

   ResourceMark rm;

+  VMInspectionErrorCallback on_error;
+  VMErrorCallbackMark mark(&on_error);
+
 #define HANDLE_THREAD(T)                                                        \
   if (T != nullptr && vma_touches_thread_stack(from, to, T)) {                  \
     print_thread_details((uintx)(T->osthread()->thread_id()), T->name(), st);   \


Note that in this context we do have a ResourceMark, so it should be fine to print the name.

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

Marked as reviewed by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21102#pullrequestreview-2317275732