[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results
From: Thomas_Stüfe <thomas.stuefe () gmail ! com>
Date: 2017-06-19 9:34:36
Message-ID: CAA-vtUzb8QuzEB8wJp6FsSJ0Jg+7aHZXCzQ7KzW_9J1b4pdWSA () mail ! gmail ! com
[Download RAW message or body]
Thank you Christoph!
On Mon, Jun 19, 2017 at 11:31 AM, Langer, Christoph <
christoph.langer@sap.com> wrote:
> Hi Thomas,
>
>
>
> this looks good to me. You still need to update the current year in the
> copyright header.
>
>
>
> Best regards
>
> Christoph
>
>
>
> *From:* serviceability-dev [mailto:serviceability-dev-
> bounces@openjdk.java.net] *On Behalf Of *Thomas St=C3=BCfe
> *Sent:* Donnerstag, 15. Juni 2017 08:31
> *To:* serviceability-dev@openjdk.java.net
> *Subject:* Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead
> to crashes or invalid results
>
>
>
> Hi Folks,
>
>
>
> may I please have a second reviewer?
>
>
>
> Thank you!
>
>
>
> Kind Regards, Thomas
>
>
>
> On Tue, Jun 13, 2017 at 11:16 PM, serguei.spitsyn@oracle.com <
> serguei.spitsyn@oracle.com> wrote:
>
> Hi Thomas,
>
> The fix looks great to me.
> Thank you for identifying and fixing the problem!
>
> Thanks,
> Serguei
>
>
>
>
> On 6/13/17 06:55, Thomas St=C3=BCfe wrote:
>
> Ping... Anyone?
>
>
>
> On Thu, Jun 1, 2017 at 2:18 PM, Thomas St=C3=BCfe <thomas.stuefe@gmail.co=
m>
> wrote:
>
> Hi all,
>
>
>
> please take a look at this proposed fix for a theoretical race in the jdw=
p
> library.
>
>
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8181419
>
> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
> 8181419-Race-in-jdwp-invoker-handling-may-lead-to-crashes-
> or-invalid-results/webrev.00/webrev/
>
>
>
> In short, this is an addition to Severin's fix to the jdwp invoke handlin=
g
> (https://bugs.openjdk.java.net/browse/JDK-8153711).
>
>
>
> We have a potential race condition where the delayed cleanup of the saved
> returnvalue object reference and the exception reference (released in
> deletePotentiallySavedGlobalRefs() ) may be overtaken by a new request
> which populates the thread request structure anew. If this happens,
> deletePotentiallySavedGlobalRefs() may actually release the return value
> / exception references of the follow up request, if that one was already
> processed.
>
>
>
> The solution I choose is safe and conservative. We still release both
> references, but use the locally saved JNI references. We just avoid
> accessing the thread local request structure after it has been cleared fo=
r
> reuse. This keeps timing and locking behaviour unchanged.
>
>
>
> I am currently running jtreg tests for com/sun/jdi on AIX and Linux.
>
>
>
> Kind Regards, Thomas
>
>
>
>
>
>
>
[Attachment #3 (text/html)]
<div dir="ltr">Thank you Christoph!</div><div class="gmail_extra"><br><div \
class="gmail_quote">On Mon, Jun 19, 2017 at 11:31 AM, Langer, Christoph <span \
dir="ltr"><<a href="mailto:christoph.langer@sap.com" \
target="_blank">christoph.langer@sap.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">
<div lang="DE" link="blue" vlink="purple">
<div class="m_2800498341106313457WordSection1">
<p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif">Hi \
Thomas,<u></u><u></u></span></p> <p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif"><u></u> \
<u></u></span></p> <p class="MsoNormal"><span lang="EN-US" \
style="font-size:11.0pt;font-family:"Calibri",sans-serif">this looks good \
to me. You still need to update the current year in the copyright \
header.<u></u><u></u></span></p> <p class="MsoNormal"><span lang="EN-US" \
style="font-size:11.0pt;font-family:"Calibri",sans-serif"><u></u> \
<u></u></span></p> <p class="MsoNormal"><span lang="EN-US" \
style="font-size:11.0pt;font-family:"Calibri",sans-serif">Best \
regards<u></u><u></u></span></p> <p class="MsoNormal"><span lang="EN-US" \
style="font-size:11.0pt;font-family:"Calibri",sans-serif">Christoph<u></u><u></u></span></p>
<p class="MsoNormal"><a name="m_2800498341106313457__MailEndCompose"><span \
lang="EN-US"><u></u> <u></u></span></a></p> <span></span>
<div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #e1e1e1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" \
style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span \
lang="EN-US" style="font-size:11.0pt;font-family:"Calibri",sans-serif"> \
serviceability-dev [mailto:<a \
href="mailto:serviceability-dev-bounces@openjdk.java.net" \
target="_blank">serviceability-dev-<wbr>bounces@openjdk.java.net</a>] <b>On Behalf Of \
</b>Thomas Stüfe<br> <b>Sent:</b> Donnerstag, 15. Juni 2017 08:31<br>
<b>To:</b> <a href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.<wbr>java.net</a><br> <b>Subject:</b> Re: \
RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid \
results<u></u><u></u></span></p> </div>
</div><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">Hi Folks,<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">may I please have a second reviewer?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Thank you!<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Kind Regards, Thomas<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Tue, Jun 13, 2017 at 11:16 PM, <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:<u></u><u></u></p> \
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0cm 0cm 0cm \
6.0pt;margin-left:4.8pt;margin-right:0cm"> <div>
<div>
<p class="MsoNormal">Hi Thomas,<br>
<br>
The fix looks great to me.<br>
Thank you for identifying and fixing the problem!<br>
<br>
Thanks,<br>
Serguei<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><br>
<br>
<br>
On 6/13/17 06:55, Thomas Stüfe wrote:<u></u><u></u></p>
</div>
</div>
</div>
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">Ping... Anyone? <u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe <<a \
href="mailto:thomas.stuefe@gmail.com" target="_blank">thomas.stuefe@gmail.com</a>> \
wrote:<u></u><u></u></p> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">Hi all, <u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">please take a look at this proposed fix for a theoretical race \
in the jdwp library.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Issue: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8181419" \
target="_blank">https://bugs.openjdk.<wbr>java.net/browse/JDK-8181419</a><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">webrev: <a \
href="http://cr.openjdk.java.net/%7Estuefe/webrevs/8181419-Race-in-jdwp-invoker-handling-may-lead-to-crashes-or-invalid-results/webrev.00/webrev/" \
target="_blank">http://cr.openjdk.<wbr>java.net/~stuefe/webrevs/<wbr>8181419-Race-in-j \
dwp-invoker-<wbr>handling-may-lead-to-crashes-<wbr>or-invalid-results/webrev.00/<wbr>webrev/</a><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">In short, this is an addition to Severin's fix to the jdwp \
invoke handling (<a href="https://bugs.openjdk.java.net/browse/JDK-8153711" \
target="_blank">https://bugs.openjdk.java.<wbr>net/browse/JDK-8153711</a>).<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">We have a potential race condition where the delayed cleanup of \
the saved returnvalue object reference and the exception reference (released in \
<wbr>deletePotentiallySavedGlobalRe<wbr>fs() ) may be overtaken by a new request \
which populates the thread request structure anew. If this happens, \
deletePotentiallySavedGlobalRe<wbr>fs() may actually release the return value / \
exception references of the follow up request, if that one was already \
processed.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">The solution I choose is safe and conservative. We still release \
both references, but use the locally saved JNI references. We just avoid accessing \
the thread local request structure after it has been cleared for reuse. This keeps \
timing and locking behaviour unchanged.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">I am currently running jtreg tests for com/sun/jdi on AIX and \
Linux.<u></u><u></u></p> </div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Kind Regards, Thomas<u></u><u></u></p>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</blockquote>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div></div></div>
</div>
</div>
</blockquote></div><br></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic