RFR 8174954: Parameter target type is allowed access after a module read edge or a package export has occurred after failed resolution

[harold seigel]

Hi John,

I used a lock because, if resolution fails, the thread checks if another 
thread's resolution either simultaneously succeeded or failed.  I didn't 
see how to safely check both success and failure by another thread 
without using a lock.

The possibility of successful resolution by one thread while another 
thread's resolution failed with a LinkageError exception is unlikely 
(probably requiring a perfectly timed change to the module graph).  
Could the check for success be done outside of the lock, after the cas?  
The resolution_error array element could then be cleared, if need be.

Thanks, Harold


On 10/20/2017 1:48 AM, John Rose wrote:
> The resolved_references array never has a null value in it except before resolution. Therefore you can use a cas instead of the more expensive lock to race and set the array elements. Why use a lock for this purpose?
>
>> On Oct 19, 2017, at 10:15 PM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> Hi Harold,
>>
>>> On 19/10/2017 12:20 AM, harold seigel wrote:
>>> Hi David,
>>> Thanks for looking at this.
>>> Please see inline comments.
>>> Thanks, Harold
>>>> On 10/17/2017 10:22 PM, David Holmes wrote:
>>>> Hi Harold,
>>>>
>>>>> On 17/10/2017 12:20 AM, harold seigel wrote:
>>>>> Hi,
>>>>>
>>>>> Please review this new JDK-10 fix for bug JDK-8174954.  If the initial resolution attempt fails with a LinkageError exception then the fix saves the exception in the resolution_errors table and sets a flag in the constant pool Cache, indicating the failure.  Subsequent attempts to do the same resolution will see that the flag is set, retrieve the exception from the resolution_errors table, and throw it, instead of re-trying the resolution.
>>>> I'm a little confused. The fix seems all about indy, which seems to be the topic of:
>>>>
>>>>   JDK-8174942 Bootstrap Method Called Multiple Times Despite Initial Resolution Failure
>>>>
>>>> whereas this bug seems to be about a more general problem. Is this fix addressing both issues??
>>> Thanks for noticing this.  This fix addresses both bugs.  The sample program for 8174942 is one of the test cases for this fix.  I'll close 8174942 as a duplicate.
>> I'm still somewhat confused as it seems that all the substantive changes here have the word "indy" in them. What part of the change actually fixes the non-indy issue tested by MethodAccessReadTwice.java?
>>
>> Thanks,
>> David
>>>> BTW there's no reason for JDK-8174954 to remain confidential.
>>> Agreed.  I opened the bug.
>>>> Thanks,
>>>> David
>>>>
>>>>> The fix also prevents a race condition if two or more threads try to do the same resolution concurrently. When a thread tries to record the result of its resolution attempt, it will check to see if another thread recorded its result first.  If so, then it will use the result recorded by the other thread.  Otherwise, it will record and use its result. Access to the resolution result is protected by the 'resolved_references' lock.
>>>>>
>>>>> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8174954.1/webrev/
>>>>>
>>>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8174954
>>>>>
>>>>> The fix was tested with the JCK Lang and VM tests, the JTreg hotspot, java/io, java/lang, java/util, and other tests, the co-located NSK tests, JPRT, with Mach5 tier2 - tier5 tests, and by hand to check race condition handling.
>>>>>
>>>>> Thanks, Harold
>>>>>