Shenandoah: Question about asserts

[Zhengyu Gu]

Hi Per,

Shenandoah heap dump was designed to be single-threaded. It looks like 
that JDK-8237354 breaks that.

Feel free to remove the assertions to unblock your work. I will file a 
bug to fix Shenandoah heap dump.

Thanks,

-Zhengyu

On 9/9/21 5:48 AM, Per Liden wrote:
> Hi Shenandoah-folks,
> 
> When doing some random testing of 
> https://github.com/openjdk/jdk/pull/5410 I noticed that Shenandoah ran 
> into a few asserts during heap dumping. The asserts check that we're 
> executing in the VM thread. However, I wonder if these asserts might be 
> a bit too aggressive, or if there's a reason these functions must only 
> be executed by the VM thread? I couldn't see any obvious reason, so I 
> simply removed the offending asserts (see patch below) and nothing 
> obvious seems to break (heap dumping tests now pass, etc).
> 
> So, would you mind if I remove these asserts, or are they really needed 
> for some reason I've missed?
> 
> cheers,
> Per
> 
> 
> diff --git a/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp 
> b/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp
> index ca9afd7ad4f..623ca9574a2 100644
> --- a/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp
> @@ -355,7 +355,6 @@ 
> ShenandoahCodeRootsIterator::ShenandoahCodeRootsIterator() :
>           _par_iterator(CodeCache::heaps()),
>           _table_snapshot(NULL) {
>     assert(SafepointSynchronize::is_at_safepoint(), "Must be at 
> safepoint");
> -  assert(!Thread::current()->is_Worker_thread(), "Should not be 
> acquired by workers");
>     CodeCache_lock->lock_without_safepoint_check();
>     _table_snapshot = 
> ShenandoahCodeRoots::table()->snapshot_for_iteration();
>   }
> diff --git a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp 
> b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
> index c0469a71d7b..261920a9d7a 100644
> --- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
> @@ -258,7 +258,6 @@ 
> ShenandoahHeapIterationRootScanner::ShenandoahHeapIterationRootScanner() :
>    }
> 
>    void ShenandoahHeapIterationRootScanner::roots_do(OopClosure* oops) {
> -   assert(Thread::current()->is_VM_thread(), "Only by VM thread");
>      // Must use _claim_none to avoid interfering with concurrent CLDG 
> iteration
>      CLDToOopClosure clds(oops, ClassLoaderData::_claim_none);
>      MarkingCodeBlobClosure code(oops, 
> !CodeBlobToOopClosure::FixRelocations);
> diff --git 
> a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.inline.hpp 
> b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.inline.hpp
> index 3f614d1e208..b2897d380aa 100644
> --- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.inline.hpp
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.inline.hpp
> @@ -86,9 +86,8 @@ ShenandoahClassLoaderDataRoots<CONCURRENT, 
> SINGLE_THREADED>::ShenandoahClassLoad
>       ClassLoaderDataGraph_lock->lock();
>     }
> 
> -  // Non-concurrent mode only runs at safepoints by VM thread
> +  // Non-concurrent mode only runs at safepoints
>     assert(CONCURRENT || SafepointSynchronize::is_at_safepoint(), "Must 
> be at a safepoint");
> -  assert(CONCURRENT || Thread::current()->is_VM_thread(), "Can only be 
> done by VM thread");
>   }
> 
>   template <bool CONCURRENT, bool SINGLE_THREADED>
>