RFR(xs): 8213834: JVMTI ResourceExhausted should not be posted in CompilerThread

[serguei.spitsyn at oracle.com]

Hi Thomas,

+1

Thanks,
Serguei


On 11/26/18 06:53, Daniel D. Daugherty wrote:
> On 11/26/18 8:07 AM, Thomas Stüfe wrote:
>> Hi guys,
>>
>> latest webrev: 
>> http://cr.openjdk.java.net/~stuefe/webrevs/8213834-jvmti-reourceexhausted-shall-not-be-posted-from-compiler-thread/webrev.02/webrev/
>
> src/hotspot/share/prims/jvmtiExport.cpp
>     No comments.
>
> Thumbs up.
>
> Dan
>
>>
>> Back to can_call_java(), since this seems to be the consensus, with a
>> comment added.
>>
>> As for the Thread::can_send_jvmti_events() property idea, I created a
>> RFE to track discussions around this:
>> https://bugs.openjdk.java.net/browse/JDK-8214294 but decided to
>> postpone this for later. I would like to close that particular issue,
>> if possible with a minimal fix which can be easily downported to older
>> released.
>>
>> Thanks, Thomas
>>
>>
>>
>>
>> On Mon, Nov 19, 2018 at 1:00 PM Thomas Stüfe 
>> <thomas.stuefe at gmail.com> wrote:
>>> Hi all,
>>>
>>> David and JC already outlined the options we have nicely.
>>>
>>> I'd like to add that I do not favor the ServiceThread delayed
>>> deliverance since a common reaction to ResourceExhausted would to
>>> print call stack of the thread running into it or to print a heap
>>> histogram, as jvmkill does. For the former only a synchronous event
>>> delivery makes sense, for the latter at least it helps analyzing.
>>>
>>> In cloud foundry, the heap histogram produced by the JVMTI agent can
>>> be helpful, and since in the majority of cases do not entail the
>>> compiler thread running out of metaspace, I'd rather preserve this
>>> ability. So to me, masking this event for this one case is the most
>>> pragmatic approach.
>>>
>>> The other option would be not to change the code but to add, in the
>>> JVMTI documentation for ResourceExhausted, the same disclaimer as for
>>> ObjectFree, GCStarted etc: "The event handler must not use JNI
>>> functions and must not use JVM TI functions except those which
>>> specifically allow such use". Then, writers of JVMTI agents like
>>> jvmkill would have to update their agents accordingly.
>>>
>>> FWIW, I think JCs idea of exposing the can_call_java attribute somehow
>>> to the outside would also work. But unfortunately not retroactively,
>>> for older releases. Whereas a simple internal patch like "mask
>>> ResourceExhausted" could be backported easily to older releases.
>>>
>>> Best Regards, Thomas
>>>
>>>
>>> On Thu, Nov 15, 2018 at 5:32 AM JC Beyler <jcbeyler at google.com> wrote:
>>>> For what it's worth I wonder if skipping the event is the best; it 
>>>> is the easiest to ensure non breaking the agent; it does not really 
>>>> go against what the spec is saying and another thread that CAN call 
>>>> java will most likely hit the issue and then all will be good in 
>>>> the world.
>>>>
>>>> However, also for what it's worth I wonder if deferring is not that 
>>>> hard to accomplish either. There already is the infrastructure for 
>>>> this and we should be relatively able to do it. Only question I 
>>>> would have is can the service thread create a JNIEnv for the event 
>>>> but I don't think that's an issue, is it?
>>>>
>>>> It might however conflict with the description of the JNIEnv in the 
>>>> spec which says the jni_env is "The JNI environment of the event 
>>>> (current) thread" though it doesn't say current of what or the 
>>>> event thread of what really.
>>>>
>>>> However, skipping it also kind of goes against the spec since it 
>>>> says: "Sent when a VM resource needed by a running application has 
>>>> been exhausted".Though someone could argue it doesn' t say it is 
>>>> sent when the resource was first noticed to be exhausted.
>>>>
>>>> So, if I over-read the spec and look at options, it seems that:
>>>> - Sending the event via the compiler thread will risk breaking 
>>>> things if the agent calls Java    -> not really an option
>>>> - Using the service thread breaks what David calls the synchronous 
>>>> assumption of the event
>>>> - Skipping the event kind of breaks the sentence that the event is 
>>>> sent when a VM resource has been exhausted
>>>>
>>>> So we come back to probably skipping is the best solution since at 
>>>> least the event remains "synchronous" when you get it.
>>>>
>>>> (A side note would be perhaps to augment ThreadInfo* with the 
>>>> "can_call_java" bit and then put in the right spots of the spec 
>>>> that only threads marked as "can_call_java" can safely call Java).
>>>>
>>>> Thanks,
>>>> Jc
>