RFR 8208303: Track JNI failures and fail tests
Alex Menkov
alexey.menkov at oracle.com
Wed Aug 8 21:13:00 UTC 2018
Hi Jc,
The fix looks good to me too.
minor note:
564 if (JNI_ENV_PTR(jni)->ExceptionOccurred(JNI_ENV_ARG(jni))) {
565 fprintf(stderr, "fill_native_frames: Exception in jni
NewWeakGlobalRef\n");
566 }
I'd expect error message mention "event_storage_add" instead
"fill_native_frames"
No need to resent webrev.
--alex
On 08/08/2018 10:56, JC Beyler wrote:
> Hi Serguei,
>
> Sounds good to me. If someone else could review it, I could add the
> metadata to the webrev and, once testing (as you say) confirms nothing
> is broken, we could push it in.
>
> If someone is motivated, that would be awesome and much appreciated!
> Jc
>
> On Wed, Aug 8, 2018 at 12:08 AM serguei.spitsyn at oracle.com
> <mailto:serguei.spitsyn at oracle.com> <serguei.spitsyn at oracle.com
> <mailto:serguei.spitsyn at oracle.com>> wrote:
>
> Hi Jc,
>
> This looks good to me especially because we discussed it a lot
> internally.
> The testing looks pretty good too.
> I submitted mach5 jobs for HepMonitor tests repeating 100 times and
> also normal tier1, tier2, hs-tier2-5.
> But it was not tested with the tiers 6 & 7 yet (most likely, need to
> make sure they run well too).
>
> Thanks,
> Serguei
>
>
> On 8/6/18 08:27, JC Beyler wrote:
>> Hi all,
>>
>> I updated the version and Serguei tested it for me so this change
>> should not change the stability of the tests. This has the
>> advantage of not having to complicate the Java tests at all, while
>> adding the post-JNI call checks.
>>
>> Webrev: http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.04/
>> <http://cr.openjdk.java.net/%7Ejcbeyler/8208303/webrev.04/>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8208303
>>
>> Thanks all for your reviews!
>> Jc
>>
>> On Fri, Jul 27, 2018 at 4:36 PM JC Beyler <jcbeyler at google.com
>> <mailto:jcbeyler at google.com>> wrote:
>>
>> Hi all,
>>
>> I did the new version that calls FatalError if JNI fails a
>> call. This has the advantage of not having to complicate the
>> Java tests at all, while adding the post-JNI call checks.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.03/
>> <http://cr.openjdk.java.net/%7Ejcbeyler/8208303/webrev.03/>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8208303
>>
>> Thanks all!
>> Jc
>>
>> On Thu, Jul 26, 2018 at 12:52 PM Chris Plummer
>> <chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>>
>> wrote:
>>
>> I'm pretty sure changes that only affect tests can be any
>> priority. But still, be a lot more cautious the closer we
>> get to release.
>>
>> Chris
>>
>> On 7/26/18 12:15 PM, Daniel D. Daugherty wrote:
>>> We entered RDP2 today (07.26). So only P1 and P2 bug
>>> fixes allowed.
>>>
>>> Dan
>>>
>>>
>>> On 7/26/18 3:14 PM, serguei.spitsyn at oracle.com
>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>> Yes, of course it has to be well tested before the push.
>>>> Does it make sense to plan it to push to 11 (after th
>>>> testing is done)?
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 7/26/18 12:08, Daniel D. Daugherty wrote:
>>>>> Please make sure this fix is well tested in Mach5 prior
>>>>> to pushing.
>>>>> In particular, I'm focused on reducing the noise in
>>>>> Mach5 tier[1-3]
>>>>> so adding any new failures there will make me grumpy :-)
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>> On 7/26/18 3:03 PM, JC Beyler wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> With the FatalError idea, here is the webrev to
>>>>>> consider, note it no longer changes the tests. If a
>>>>>> JNI call fails, then we call FatalError.
>>>>>>
>>>>>> Let me know what you think:
>>>>>>
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.01/ <http://cr.openjdk.java.net/%7Ejcbeyler/8208303/webrev.01/>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8208303
>>>>>>
>>>>>> Thanks!
>>>>>> Jc
>>>>>>
>>>>>> On Thu, Jul 26, 2018 at 10:46 AM
>>>>>> serguei.spitsyn at oracle.com
>>>>>> <mailto:serguei.spitsyn at oracle.com>
>>>>>> <serguei.spitsyn at oracle.com
>>>>>> <mailto:serguei.spitsyn at oracle.com>> wrote:
>>>>>>
>>>>>> Hi Jc,
>>>>>>
>>>>>> Good idea.
>>>>>> I was thinking about something like this.
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>
>>>>>> On 7/26/18 10:40, JC Beyler wrote:
>>>>>>> Hi Serguei,
>>>>>>>
>>>>>>> As I was looking at another test bug
>>>>>>> (https://bugs.openjdk.java.net/browse/JDK-8191519);
>>>>>>> the proposal for that bug is to have a JNI call
>>>>>>> to FatalError to provoke a failure.
>>>>>>>
>>>>>>> If we went down that route, this webrev is
>>>>>>> simpler, no? Instead of setting failure_status
>>>>>>> and checking it later; just fail fatally and be
>>>>>>> done with it, no? That way, the tests in Java
>>>>>>> land don't have to be changed actually, no?
>>>>>>>
>>>>>>> What would we prefer for tests? Remember there
>>>>>>> was a failure and test it later or fail fast via
>>>>>>> JNI's FatalError?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Jc
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Jul 26, 2018 at 10:04 AM
>>>>>>> serguei.spitsyn at oracle.com
>>>>>>> <mailto:serguei.spitsyn at oracle.com>
>>>>>>> <serguei.spitsyn at oracle.com
>>>>>>> <mailto:serguei.spitsyn at oracle.com>> wrote:
>>>>>>>
>>>>>>> Hi Jc,
>>>>>>>
>>>>>>> It looks good to me.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>
>>>>>>> On 7/26/18 09:58, JC Beyler wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> The tests in the HeapMonitor subsystem has a
>>>>>>>> lot of JNI calls. There is a need for
>>>>>>>> verification and testing if anything in the
>>>>>>>> JNI subsystem failed unexpectedly.
>>>>>>>>
>>>>>>>> Here is a webrev that tracks if a JNI call
>>>>>>>> does fail and the tests will fail if any JNI
>>>>>>>> call does fail.
>>>>>>>>
>>>>>>>> Could I have a few reviews please for:
>>>>>>>> Webrev:
>>>>>>>> http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.00/
>>>>>>>> <http://cr.openjdk.java.net/%7Ejcbeyler/8208303/webrev.00/>
>>>>>>>> Bug:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8208303
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Jc
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Jc
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>>
>>>>>> Thanks,
>>>>>> Jc
>>>>>
>>>>
>>>
>>
>>
>>
>> --
>>
>> Thanks,
>> Jc
>>
>>
>>
>> --
>>
>> Thanks,
>> Jc
>
>
>
> --
>
> Thanks,
> Jc
More information about the serviceability-dev
mailing list