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

[coleen.phillimore at oracle.com]

On 7/22/20 4:21 AM, Thomas Schatzl wrote:
> Hi,
>
> On 22.07.20 02:42, David Holmes wrote:
>> Hi Thomas,
>>
>> I've looked at the incremental update and I am happy with that.
>
> In the response to Serguei's review there were some comment updates 
> and new webrevs.
>
>>
>> I also, prompted by you mentioning it, took a deeper look at the 
>> biased-locking code to ensure it also keeps the MonitorInfo's 
>> thread-confined, and to see whether the handshake versions could 
>> themselves be susceptible to interference from safepoints (which they 
>> can't as far as I can determine). And that all seems fine.
>
> Thanks for looking at this again in more detail. I couldn't spot 
> problems either, but this is not code I am proficient with.
>
>> As per offline discussions I know that there has been an alternate 
>> proposal for a completely localized fix in the stackwalker code that 
>> simply retrieves the list of monitors, uses the length to create the 
>> array, then re-retrieves the list of monitors to populate the array 
>> (the length of which can't change as we are dealing with the current 
>> thread). My only concern with that approach is the performance impact 
>> if we have deep stacks with lots of monitors. There is a 
>> microbenchmark for StackWalker in the repo:
>>
>> open/test/micro/org/openjdk/bench/java/lang/StackWalkBench.java
>>
>> but it doesn't test anything to do with monitor usage.
>
> Ultimately I am good with either change, as long as it's being fixed. 
> I initially thought about this solution too, but had the same 
> concerns. Stacks can be really deep particularly with some frameworks 
> (maybe not fully materialized) but idk the actual impact of doing the 
> walk twice.
>
> Potentially you could have different fixes for different versions.

Yes.  These patches look good to me for JDK 15 and 16.  We'll open an 
RFE to consider the alternate patch after more performance testing for 
future versions, but this fixes the problem and will not be difficult to 
backport to JDK 11.

Thank you!
Coleen
>
> Thanks,
>   Thomas