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

[Mikhailo Seledtsov]

+1,

Thank you,
Misha

On 2/5/18, 10:47 AM, serguei.spitsyn at oracle.com wrote:
> +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
>>>>>>
>>>>
>