Hi David, > I see little point in subclassing NonJavaThread (via NamedThread) but > then overriding pre_run() and post_run() so that you don't do anything > that NonJavaThread is supposed to do regarding the NJT iterator > capabilities.
The problem is the post_run() method of NamedThread calls Thread::clear_thread_current(), which then makes it impossible to delete the thread at least in a debug build, since the code in ~Thread calls os::free_thread() which calls Thread::current()->->osthread() in an assert, which obviously will crash. Originally I tried not use my own threads at all and instead use the WorkGang from CollectedHeap:: get_safepoint_workers(). But this ultimately failed because I'm not allowed to iterate the heap in a worker thread on Shenandoah. Additionally ParallelGC did not implement get_safepoint_workers(), but that should have not been a problem. Maybe it is better to try to get this to work (e.g. if I could specify a foreground task when calling run_task(), the problem could be avoid by doing the iteration in the foreground task). But I'm not sure how changes in this area are seen. > For your monitor operations, you should use a MonitorLocker and then > call ml->wait() which will do the right thing with respect to "no > safepoint checks" without you needing to specify it directly. Thanks, will do. Best regards, Ralf -----Original Message----- From: David Holmes <david.hol...@oracle.com> Sent: Dienstag, 11. Februar 2020 08:44 To: Schmelter, Ralf <ralf.schmel...@sap.com>; Yasumasa Suenaga <suen...@oss.nttdata.com>; OpenJDK Serviceability <serviceability-dev@openjdk.java.net> Cc: yasue...@gmail.com Subject: Re: RFR(L) 8237354: Add option to jcmd to write a gzipped heap dump Hi again Ralf, :) A few more comments after taking a closer look at the thread code. On the surface it seems to me this is a case where it would be okay to introduce a subclass of Thread that is not JavaThread nor NonJavaThread. I see little point in subclassing NonJavaThread (via NamedThread) but then overriding pre_run() and post_run() so that you don't do anything that NonJavaThread is supposed to do regarding the NJT iterator capabilities. But we currently expect all threads to fit into one category or another, so this is problematic. :( I thinking disabling the NJT functionality is also problematic. So not sure what to suggest yet. BTW you extended NamedThread but you never actually set a name AFAICS. ?? For your monitor operations, you should use a MonitorLocker and then call ml->wait() which will do the right thing with respect to "no safepoint checks" without you needing to specify it directly. Cheers, David
