RFR: 8166188: G1 Needs pre barrier on dereference of weak JNI handles

[Mikael Gerdin]

Hi Martin,

Thanks for taking a look at this change.

On 2017-01-23 19:46, Doerr, Martin wrote:
> Hi everybody,
>
> thanks for addressing this issue and for the webrev.
>
> While reading code, I was wondering if the result handler for oop types ever gets called (except on PPC64).
> Looks like it is only there to compare the pointer after the native call and do something special in this case instead of calling it.
>
> Wouldn't it be cleaner to use the result handler and take care of the G1 pre barrier there?

Can't say I know much about the result handlers in general but I get the 
feeling that they are only there for the interpreter.
Are the result handlers applied to the return values of compiled native 
wrappers as well?

Thanks
/Mikael

>
> Btw. PPC64 currently uses a different implementation which really calls the oop result handler. The native entry saves the handle to the frame's lresult field and sets the oop_temp to NULL. The oop result handler takes care of the unboxing. In addition frame::interpreter_frame_result should use JNIHandles::resolve to retrieve the oop (which is not yet implemented as it should be). Also, one has to make sure that the active handles don't get reset too early.
> This implementation doesn't force the oop to be live. GC may still set it to NULL if it decides to kill the jweak.
>
> I think PPC64 and the other platforms should do it the same way. The question is what's the better approach.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: s390x-port-dev [mailto:s390x-port-dev-bounces at openjdk.java.net] On Behalf Of Kim Barrett
> Sent: Donnerstag, 19. Januar 2017 19:05
> To: Mikael Gerdin <mikael.gerdin at oracle.com>
> Cc: s390x-port-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net; hotspot-dev developers <hotspot-dev at openjdk.java.net>
> Subject: Re: RFR: 8166188: G1 Needs pre barrier on dereference of weak JNI handles
>
>> On Jan 19, 2017, at 11:28 AM, Mikael Gerdin <mikael.gerdin at oracle.com> wrote:
>>
>> Hi Kim,
>>
>> On 2017-01-19 06:31, Kim Barrett wrote:
>>> Please review this change to jweak to support G1.
>>>
>>> [...]
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8166188
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~kbarrett/8166188/hotspot.02/
>>
>> I thought a bit about this issue today and unfortunately I've uncovered yet another location where a weak JNI handle needs to have its weak tag cleared.
>> Consider the following JNI code snippet
>>
>> [...]
>> I've created a test to provoke this situation and put it at:
>> http://cr.openjdk.java.net/~mgerdin/8166188-pass-arguments/webrev/
>> run the test with
>> $ make test-hotspot-jtreg-native
>>
>> Sorry for not coming up with this sooner, I just realized this problem today.
>
> Well, I'm happy you found it before this went out the door.  Despite looking for this sort of thing, I completely missed this one.  I need to re-review casts to oop*.
>