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

[Tobias Hartmann]

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