RFR: 8264149 BreakpointInfo::set allocates metaspace object in VM thread [v2]

[Coleen Phillimore]

On Tue, 30 Mar 2021 01:17:27 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Only allow non-TRAPS version of Metaspace::allocate at a safepoint (or by non-Java thread)
>
> src/hotspot/share/oops/method.cpp line 570:
> 
>> 568:   if (current->is_Java_thread()) {
>> 569:     // For when TRAPS is JavaThread.
>> 570:     counters = MethodCounters::allocate(mh, current->as_Java_thread());
> 
> I'm not at all clear what we are doing here and it seems premature to anticipate the TRAPS change to JavaThread. The code will need reworking for that change because you still check for a pending exception regardless of what type of Thread current is.
> I don't see why we need to call a TRAPS version of allocate when we are going to clear any exception - just call the non-traps version (and remove the assert you added). Just because we are in a JavaThread it doesn't mean we have to throw exceptions.

With this change, if the thread is not a Java thread, say the VMThread, we cannot throw an exception because we can't allocate a Java object for that exception. That's why we call the non-TRAPS version for !JavaThread.  So there's no need to check for a pending exception because it can't throw one. Also, eventually we want to enforce that only JavaThreads can throw/catch Java exceptions.
The change requested by Thomas was that if we *can* thrown an exception, we should call the TRAPS version because that version will adjust the GC threshold.  We can't call GC with the non-TRAPS version.
So this is the simplest thing.  We have two versions of this function (the TRAPS version calls the non-TRAPS version). The non-TRAPS version is the special one that can be used by the VMThread for allocating metadata, all other callers stay the same.
I can remove the as_Java_thread() and line 569 and you can add it back when we're able to change TRAPS to JavaThread.

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

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