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

[Tobias Hartmann]

Thanks everyone!

Best regards,
Tobias

On 05.02.2018 20:14, Mikhailo Seledtsov wrote:
> +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
>>>>>>>
>>>>>
>>