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

[David Holmes]

+1 more

Thanks,
David

On 27/11/2018 3:57 am, serguei.spitsyn at oracle.com wrote:
> 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
>>
>