RFR: 8253833: mutexLocker assert_locked_or_safepoint should not access VMThread state from non-VM-thread

[Daniel D.Daugherty]

On Thu, 8 Oct 2020 15:57:22 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> It's unsafe for all threads except VM thread to access the current vm operation.
>> This part of the assert is also faulty:
>> If we are not at safepoint and the operation requester (calling thread) would be the owner of the lock do not mean it
>> is safe for current thread.
>> Passes t1-5. (also note VMThread::vm_operation() assert current thread is VM thread, and I have seen no such assert)
>> 
>> Thanks
>
> src/hotspot/share/runtime/mutexLocker.cpp line 171:
> 
>> 169:   if (lock->owned_by_self()) return;
>> 170:   if (SafepointSynchronize::is_at_safepoint()) return;
>> 171:   if (!Universe::is_fully_initialized()) return;
> 
> Is this bailout on L171 for the code you are deleting (old L172-4)
> or for the fatal() on old L175?

I was hope that "git blame" would help, but most of this code is ancient:

da18495f385b (Coleen Phillimore 2019-08-22 09:51:36 -0400 166) void assert_locked_or_safepoint(const Mutex* lock) {
8153779ad32d (J. Duke           2007-12-01 00:00:00 +0000 167)   // check if this thread owns the lock (common case)
8153779ad32d (J. Duke           2007-12-01 00:00:00 +0000 168)   assert(lock != NULL, "Need non-NULL lock");
8153779ad32d (J. Duke           2007-12-01 00:00:00 +0000 169)   if (lock->owned_by_self()) return;
8153779ad32d (J. Duke           2007-12-01 00:00:00 +0000 170)   if (SafepointSynchronize::is_at_safepoint()) return;
8153779ad32d (J. Duke           2007-12-01 00:00:00 +0000 171)   if (!Universe::is_fully_initialized()) return;
8153779ad32d (J. Duke           2007-12-01 00:00:00 +0000 172)   // see if invoker of VM operation owns it
8153779ad32d (J. Duke           2007-12-01 00:00:00 +0000 173)   VM_Operation* op = VMThread::vm_operation();
8153779ad32d (J. Duke           2007-12-01 00:00:00 +0000 174)   if (op != NULL && op->calling_thread() ==
lock->owner()) return; 1e71f6773659 (David Lindholm    2015-09-29 11:02:08 +0200 175)   fatal("must own lock %s",
lock->name()); 8153779ad32d (J. Duke           2007-12-01 00:00:00 +0000 176) }

Update: Stripped the filename from the "git blame" output so it wasn't quite so wide...
Update: Changing to a `code` quote helped the formatting a bit...

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

PR: https://git.openjdk.java.net/jdk/pull/563