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

[David Holmes]

On 9/10/2020 4:21 pm, Robbin Ehn wrote:
> On Thu, 8 Oct 2020 22:02:34 GMT, David Holmes <dholmes 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 170:
>>
>>> 168:   assert(lock != NULL, "Need non-NULL lock");
>>> 169:   if (lock->owned_by_self()) return;
>>> 170:   if (SafepointSynchronize::is_at_safepoint()) return;
>>
>> This is actually inadequate as it would only be safe (in a lock sneaking sense) if at a safepoint AND in the vmThread.
>> Ditto for L179.
> 
> It is also safe for threads respecting the safepoint e.g. GC safepoint worker (safepoint cannot end until the safepoint
> workers stops). In some cases we do not use Mutex during safepoint when executing with such thread.

True
> But an entire over-haul of these asserts and what the implicit rules are is out of scope here.

Yes and I think if we can't formulate the exact condition then logically 
we have:

(is_at_safepoint() && is_VMThread()) || (is_at_safepoint() && 
complexConditionWeHaveToAssumeIsTrue())

which simplifies to just

is_at_safepoint()

:)

Cheers,
David

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