RFR(S): 8195731: [Graal] runtime/SharedArchiveFile/serviceability/transformRelatedClasses/TransformSuperSubTwoPckgs.java intermittently fails with Graal JIT

[serguei.spitsyn at oracle.com]

+1

Thanks,
Serguei

On 2/5/18 10:42, Vladimir Kozlov wrote:
> Looks good.
>
> Thanks,
> Vladimir
>
> On 2/4/18 11:06 PM, Tobias Hartmann wrote:
>> Thanks Vladimir and Serguei!
>>
>> For the record, I'm going to push this version:
>> http://cr.openjdk.java.net/~thartmann/8195731/webrev.02/
>>
>> Best regards,
>> Tobias
>>
>> On 02.02.2018 21:56, serguei.spitsyn at oracle.com wrote:
>>> Thanks, guys!
>>> I'm Okay with this fix too.
>>> Interesting that I've just investigated similar situation in 
>>> Transformer
>>> and was puzzled why the exception was not propagated.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 2/2/18 10:26, Vladimir Kozlov wrote:
>>>> Thank you, Tobias and David.
>>>>
>>>> With this information I agree to use System.exit().
>>>> May be just add your new log("Transformation failed!"); to webrev.00
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 2/2/18 1:36 AM, Tobias Hartmann wrote:
>>>>> Hi David,
>>>>>
>>>>> On 02.02.2018 10:15, David Holmes wrote:
>>>>>> http://openjdk.java.net/jtreg/faq.html#question2.6
>>>>>>
>>>>>> 2.6. Should a test call the System.exit method?
>>>>>>
>>>>>> No. Depending on how you run the tests, you may get a security 
>>>>>> exception from the harness.
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Plus if you call System.exit you have to run in othervm mode.
>>>>>>
>>>>>> So generally we avoid System.exit and just fail by throwing an 
>>>>>> exception from "main" (or whatever the test entry point
>>>>>> is, depending on which framework it uses - like testng). There 
>>>>>> are exceptions of course (pardon the pun) and a lot of
>>>>>> legacy tests use System.exit(97) or System.exit(95) to indicate 
>>>>>> success or failure.
>>>>>
>>>>> Thanks for the pointer, that makes sense to me.
>>>>>
>>>>>>> The problem is that throwing an exception in 
>>>>>>> ClassFileTransformer::transform() is silently ignored:
>>>>>>> "If the transformer throws an exception (which it doesn't 
>>>>>>> catch), subsequent transformers will still be called and the
>>>>>>> load, redefine or retransform will still be attempted. Thus, 
>>>>>>> throwing an exception has the same effect as returning
>>>>>>> null." [1]
>>>>>>>
>>>>>>> As a result, the test fails without any information. I've 
>>>>>>> basically copied this code from
>>>>>>> runtime/RedefineTests/RedefineAnnotations.java [2] were we use 
>>>>>>> System.exit as well. If there's a reason to avoid
>>>>>>> System.exit here, we can also just print an error and fail later 
>>>>>>> with the generic exception:
>>>>>>> "java.lang.RuntimeException: 'parent-transform-check: 
>>>>>>> this-has-been--transformed' missing from stdout/stderr"
>>>>>>
>>>>>> This does sounds like a case where you need System.exit to force 
>>>>>> immediate termination.
>>>>>
>>>>> Yes, I think so too.
>>>>>
>>>>> Thanks,
>>>>> Tobias
>>>>>
>>>