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 David,

I'll add comments to the test to make it clearer that the IAE exception 
occurs when executing the invokedynamic instruction for the string concat.

Thanks, Harold

On 10/22/2017 2:17 AM, David Holmes wrote:
> Hi Harold,
>
> On 20/10/2017 11:37 PM, harold seigel wrote:
>> Hi David,
>>
>> MethodAccessReadTwice.java is an indy test.  Lines 120 - 153 test 
>> that the same invokedynamic instruction gets the same result.  The 
>> invokedynamic instruction gets generated for the string concatenation 
>> at line 31 of file .../p5/c5.java:
>
> Okay. It was totally not at all evident from the bug report and the 
> test code that the underlying problem was actually related to indy due 
> to the string concatenation.
>
> Thanks,
> David
>
>>
>>        24 package p5;
>>        25
>>        26 import java.lang.Module;
>>        27 import p2.c2;
>>        28
>>        29 public class c5 {
>>        30     public void method5(p2.c2 param) {
>>        31         System.out.println("In c5's method5 with param = " 
>> + param);
>>        32     }
>>        33
>>        34     public void methodAddReadEdge(Module m) {
>>        35         // Add a read edge from p5/c5's module (first_mod) 
>> to second_mod
>>        36         c5.class.getModule().addReads(m);
>>        37     }
>>        38 }
>>
>> The indy resolution throws an IAE because package p5's module cannot 
>> read package p2's module.
>>
>> The second test case, at lines 156 - 161 of 
>> MethodAccessReadTwice.java, similarly tests invokedynamic 
>> instructions.  In this case, they are generated for the string 
>> concatenations at lines 32 and 43 of file .../p7/c7.java.
>>
>> This problem does not exist for non-indy instructions because, when 
>> their resolutions fail, they update the constant pool. Indy cannot do 
>> this because, unlike other instructions, different indy instructions 
>> that use the same constant pool index are allowed to get different 
>> results.  Hence, the fix is indy only.
>>
>> Thanks, Harold
>>
>> On 10/20/2017 1:15 AM, David Holmes 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
>>>>>>
>>>>
>>