[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR 8208303: Track JNI failures and fail tests
From: JC Beyler <jcbeyler () google ! com>
Date: 2018-07-27 23:36:41
Message-ID: CAF9BGBxpCDesppF8uzw5ycAkmNu_xNvq-gMAg_sRHAFqU1OTPw () mail ! gmail ! com
[Download RAW message or body]
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/
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@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@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/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8208303
>
> Thanks!
> Jc
>
> On Thu, Jul 26, 2018 at 10:46 AM serguei.spitsyn@oracle.com <
> serguei.spitsyn@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@oracle.com <
>> serguei.spitsyn@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/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8208303
>>>
>>> Thanks,
>>> Jc
>>>
>>>
>>>
>>
>> --
>>
>> Thanks,
>> Jc
>>
>>
>>
>
> --
>
> Thanks,
> Jc
>
>
>
>
>
>
--
Thanks,
Jc
[Attachment #3 (text/html)]
<div dir="ltr">Hi all,<div><br></div><div>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.</div><div><br></div><div>Webrev: \
<a href="http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.03/">http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.03/</a><br></div><div><span \
style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Bug: \
</span><a href="https://bugs.openjdk.java.net/browse/JDK-8208303" target="_blank" \
style="color:rgb(17,85,204);font-size:small;background-color:rgb(255,255,255)">https:/ \
/bugs.openjdk.java.net/browse/JDK-8208303</a><br></div><div><br></div><div>Thanks \
all!</div><div>Jc</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jul \
26, 2018 at 12:52 PM Chris Plummer <<a \
href="mailto:chris.plummer@oracle.com">chris.plummer@oracle.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div class="m_-7329391508524927823moz-cite-prefix">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.<br>
<br>
Chris<br>
<br>
On 7/26/18 12:15 PM, Daniel D. Daugherty wrote:<br>
</div>
<blockquote type="cite">
<tt>We entered RDP2 today (07.26). So only P1 and P2 bug fixes
allowed.<br>
<br>
Dan<br>
<br>
</tt><br>
<div class="m_-7329391508524927823moz-cite-prefix">On 7/26/18 3:14 PM, <a \
class="m_-7329391508524927823moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a> wrote:<br> </div>
<blockquote type="cite">
<div class="m_-7329391508524927823moz-cite-prefix">Yes, of course it has to \
be well tested before the push.<br>
Does it make sense to plan it to push to 11 (after th testing
is done)?<br>
<br>
Thanks,<br>
Serguei <br>
<br>
<br>
On 7/26/18 12:08, Daniel D. Daugherty wrote:<br>
</div>
<blockquote type="cite"> <tt>Please
make sure this fix is well tested in Mach5 prior to pushing.<br>
In particular, I'm focused on reducing the noise in Mach5
tier[1-3]<br>
so adding any new failures there will make me grumpy :-)<br>
<br>
Dan<br>
<br>
</tt><br>
<div class="m_-7329391508524927823moz-cite-prefix">On 7/26/18 3:03 PM, JC \
Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi all,<br>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>Let me know what you think:</div>
<div><br>
</div>
<div>Webrev: <a \
href="http://cr.openjdk.java.net/%7Ejcbeyler/8208303/webrev.01/" \
target="_blank">http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.01/</a></div>
<div>Bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8208303" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8208303</a><br> </div>
<div><br>
</div>
<div>Thanks!</div>
<div>Jc</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Thu, Jul 26, 2018 at 10:46 AM <a \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a>
<<a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote">
<div>
<div \
class="m_-7329391508524927823m_3738914823647369337moz-cite-prefix">Hi Jc,<br>
<br>
Good idea.<br>
I was thinking about something like this.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 7/26/18 10:40, JC Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi Serguei,<br>
<div><br>
</div>
<div>As I was looking at another test bug (<a \
href="https://bugs.openjdk.java.net/browse/JDK-8191519" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8191519</a>); the proposal \
for that bug is to have a JNI call to FatalError to provoke a failure.</div>
<div><br>
</div>
<div>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?</div>
<div><br>
</div>
<div>What would we prefer for tests? Remember
there was a failure and test it later or fail
fast via JNI's FatalError?</div>
<div><br>
</div>
<div>Thanks,</div>
<div>Jc</div>
<div><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Thu, Jul 26, 2018 at 10:04 AM <a \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a>
<<a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote">
<div>
<div \
class="m_-7329391508524927823m_3738914823647369337m_-6959169999082557489moz-cite-prefix">Hi
Jc,<br>
<br>
It looks good to me.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 7/26/18 09:58, JC Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi all,<br>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>Here is a webrev that tracks if a JNI
call does fail and the tests will fail
if any JNI call does fail.</div>
<div><br>
</div>
<div>Could I have a few reviews please
for:</div>
<div>Webrev: <a \
href="http://cr.openjdk.java.net/%7Ejcbeyler/8208303/webrev.00/" \
target="_blank">http://cr.openjdk.java.net/~jcbeyler/8208303/webrev.00/</a></div> \
<div>Bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8208303" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8208303</a></div> <div><br>
</div>
<div dir="ltr" \
class="m_-7329391508524927823m_3738914823647369337m_-6959169999082557489gmail_signature">
<div dir="ltr">Thanks,
<div>Jc</div>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" \
class="m_-7329391508524927823m_3738914823647369337gmail_signature" \
data-smartmail="gmail_signature"> <div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" class="m_-7329391508524927823gmail_signature" \
data-smartmail="gmail_signature"> <div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<p><br>
</p>
</div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" \
class="gmail_signature" data-smartmail="gmail_signature"><div \
dir="ltr"><div><br></div>Thanks,<div>Jc</div></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic