[15?] RFR (S): 8249192: MonitorInfo stores raw oops across safepoints

[Daniel D. Daugherty]

Just addressing one item for now:

 > The CR has been triaged as P3, but I would like to ask whether it might
 > be good to increase its priority to P2...

The ILW set during triage is this:

 > ILW = HLM = P3

The Work around portion is "M", but I'm not aware of any work around
other than rolling the dice again. If there is no work around, then
that changes the Work around portion to "H" which makes this a P2.

Misha, please chime in on this thread about this rationale for changing
the priority from P3 -> P2. Thanks, in advance.

Dan



On 7/20/20 10:49 AM, Thomas Schatzl wrote:
> Forwarding to hotspot-dev where it belongs after wrongly sending to 
> hotspot-gc-dev.
>
>
> -------- Forwarded Message --------
> Subject: [15?] RFR (S): 8249192: MonitorInfo stores raw oops across 
> safepoints
> Date: Mon, 20 Jul 2020 12:07:38 +0200
> From: Thomas Schatzl <thomas.schatzl at oracle.com>
> To: hotspot-dev at openjdk.java.net <hotspot-gc-dev at openjdk.java.net>
>
> Hi all,
>
>   can I get some reviews to handle'ize some raw oops in the 
> MonitorInfo class?
>
> (Afaiu only) in LiveFrameStream::monitors_to_object_array() we try to 
> allocate an objArray with raw oops held in the MonitorInfo class that 
> are passed in a GrowableArray. This allocation can lead to a garbage 
> collection, with the usual random crashes.
>
> This change changes the raw oops in MonitorInfo to Handles, and adds a 
> few HandleMarks along the way to make these handles go away asap.
>
> This issue has been introduced in JDK-8140450: Implement Stack-Walking 
> API in jdk9.
>
> The CR has been triaged as P3, but I would like to ask whether it 
> might be good to increase its priority to P2 and apply for inclusion 
> in 15. My arguments are as follows:
>
> - the original issue why I started looking at this were lots of 
> seemingly random crashes (5 or 6 were reported and the change 
> temporarily backed out for this reason) in tier8 with a g1 change that 
> changed young gen sizing. These crashes including that young gen 
> sizing change are all gone now with this bugfix.
> I.e. this suggests that so far we seem to have not encountered this 
> issue more frequently due to pure luck wrt to generation sizing.
>
> - it affects all collectors (naturally).
>
> - there are quite a few user reported random crashes with IntelliJ and 
> variants, which due to the nature of IDEs tending to retrieve stack 
> traces fairly frequently would be more affected than usual. So I 
> suspect at least some of them to be caused by this issue, these are 
> the only raw oops I am aware of.
>
> My understanding of the cause and fix is fairly good, but I am no 
> expert in this area, so I would like to defer to you about this 
> suggestion. The change is imo important enough to be backported to 11 
> and 15 anyway, but the question is about the risk/reward tradeoff wrt 
> to bringing it to 15 and not 15.0.1.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8249192
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8249192/webrev/
> Testing:
> tier1-5,tier8 (with some unrelated changes), 1800+ runs of the reproducer
>
> Thanks,
>   Thomas
>