Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-19 Thread Thomas Stüfe 
Thank you Christoph!

On Mon, Jun 19, 2017 at 11:31 AM, Langer, Christoph <
christoph.lan...@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-
> boun...@openjdk.java.net] *On Behalf Of *Thomas Stüfe
> *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.spit...@oracle.com <
> serguei.spit...@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üfe wrote:
>
> Ping... Anyone?
>
>
>
> On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe 
> wrote:
>
> Hi all,
>
>
>
> please take a look at this proposed fix for a theoretical race in the jdwp
> 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 handling
> (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 for
> 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
>
>
>
>
>
>
>

RE: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-19 Thread Langer, Christoph 
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-boun...@openjdk.java.net] 
On Behalf Of Thomas Stüfe
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.spit...@oracle.com<mailto:serguei.spit...@oracle.com> 
mailto:serguei.spit...@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üfe wrote:
Ping... Anyone?

On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe 
mailto:thomas.stu...@gmail.com>> wrote:
Hi all,

please take a look at this proposed fix for a theoretical race in the jdwp 
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/<http://cr.openjdk.java.net/%7Estuefe/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 handling 
(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 for 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

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-19 Thread Thomas Stüfe 
Thank you Severin!

On Mon, Jun 19, 2017 at 11:10 AM, Severin Gehwolf 
wrote:

> Hi Thomas,
>
> On Tue, 2017-06-13 at 15:55 +0200, Thomas Stüfe wrote:
> > Ping... Anyone?
> >
> > On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe  > > wrote:
> > > Hi all,
> > >
> > > please take a look at this proposed fix for a theoretical race in
> > > the jdwp 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
> > > handling (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 for reuse. This keeps timing and locking behaviour
> > > unchanged.
> > >
> > > I am currently running jtreg tests for com/sun/jdi on AIX and
> > > Linux.
>
> The fix makes sense to me. Looks good! I'm not an OpenJDK Reviewer.
>
> Cheers,
> Severin
>

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-19 Thread Severin Gehwolf 
Hi Thomas,

On Tue, 2017-06-13 at 15:55 +0200, Thomas Stüfe wrote:
> Ping... Anyone?
> 
> On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe  > wrote:
> > Hi all,
> > 
> > please take a look at this proposed fix for a theoretical race in
> > the jdwp 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
> > handling (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 for reuse. This keeps timing and locking behaviour
> > unchanged.
> > 
> > I am currently running jtreg tests for com/sun/jdi on AIX and
> > Linux.

The fix makes sense to me. Looks good! I'm not an OpenJDK Reviewer.

Cheers,
Severin

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-17 Thread Thomas Stüfe 
Thank you Serguei!

..Thomas

On Sun 18. Jun 2017 at 00:42, serguei.spit...@oracle.com <
serguei.spit...@oracle.com> wrote:

> Hi Thomas,
>
> An update on this.
>
> I ran the nsk.jdi and jtreg jdk.jdi tests on Linux-x64.
> The results are pretty good.
> One jtreg test failed:
>  com/sun/jdi/LineNumberInfo.java: Test javac regressions in the generation
> of line number info
>
> This failure seems to be unrelated to your fix as it is about incorrect
> line number information.
>
> Thanks,
> Serguei
>
>
>
>
> On 6/15/17 11:00, serguei.spit...@oracle.com wrote:
>
> Hi Thomas,
>
> Sure.
>
>
> I'll also run some internal jdi tests.
>
> Thanks,
> Serguei
>
>
> On 6/13/17 21:17, Thomas Stüfe wrote:
>
> Hi Sergey,
>
> thank you for the review!
>
> I ran jdi jtreg tests on both AIX and Linux x64, I had some issues but
> they seemed preexisting (same issues without my patch). Could you please
> also run tests you think are necessary on your platforms? Thanks!
>
> best Regards, Thomas
>
> On Wed, Jun 14, 2017 at 1:36 AM, serguei.spit...@oracle.com <
> serguei.spit...@oracle.com> wrote:
>
>> On 6/13/17 14:16, serguei.spit...@oracle.com wrote:
>>
>> Hi Thomas,
>>
>> The fix looks great to me.
>>
>>
>> Forgot to say that the copyright comment need an update.
>>
>> Thanks,
>> Serguei
>>
>>
>> Thank you for identifying and fixing the problem!
>>
>> Thanks,
>> Serguei
>>
>>
>> On 6/13/17 06:55, Thomas Stüfe wrote:
>>
>> Ping... Anyone?
>>
>> On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe 
>> wrote:
>>
>>> Hi all,
>>>
>>> please take a look at this proposed fix for a theoretical race in the
>>> jdwp 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
>>> handling (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 for
>>> 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
>>>
>>
>>
>>
>>
>
>

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-17 Thread serguei.spit...@oracle.com 

  
  
Hi Thomas,
  
  An update on this.
  
  I ran the nsk.jdi and jtreg jdk.jdi tests on Linux-x64.
  The results are pretty good.
  One jtreg test failed:
   com/sun/jdi/LineNumberInfo.java: Test javac regressions in the
  generation of line number info 
  
  This failure seems to be unrelated to your fix as it is about
  incorrect line number information.
  
  Thanks,
  Serguei
  
  
  
  On 6/15/17 11:00, serguei.spit...@oracle.com wrote:


  
  Hi Thomas,

Sure.
I'll also run some internal jdi tests.

Thanks,
Serguei


On 6/13/17 21:17, Thomas Stüfe wrote:
  
  
Hi Sergey,
  
  
  thank you for the review! 
  
  
  I ran jdi jtreg tests on both AIX and Linux x64, I had
some issues but they seemed preexisting (same issues without
my patch). Could you please also run tests you think are
necessary on your platforms? Thanks!
  
  
  best Regards, Thomas


  On Wed, Jun 14, 2017 at 1:36 AM, serguei.spit...@oracle.com

wrote:

  
  On
6/13/17 14:16, serguei.spit...@oracle.com
wrote:
  
  
Hi
  Thomas,
  
  The fix looks great to me.

  
  
 Forgot to say that the copyright comment need an
update.

Thanks,
Serguei
  
  
  

  Thank you for identifying and fixing the problem!
  
  Thanks,
  Serguei
  
  
  On 6/13/17 06:55, Thomas Stüfe wrote:


  Ping... Anyone?

  On Thu, Jun 1, 2017
at 2:18 PM, Thomas Stüfe 
wrote:

  Hi all,


please take a look at this proposed
  fix for a theoretical race in the jdwp
  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
  handling (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
  for reuse. This keeps timing and
  locking behaviour unchanged.

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-15 Thread serguei.spit...@oracle.com 

  
  
On 6/15/17 11:00,
  serguei.spit...@oracle.com wrote:


  
  Hi Thomas,

Sur.
  


A typo: 'Sur' -> 'Sure'. :)

Thanks,
Serguei



   I'll also run some internal jdi
tests.

Thanks,
Serguei


On 6/13/17 21:17, Thomas Stüfe wrote:
  
  
Hi Sergey,
  
  
  thank you for the review! 
  
  
  I ran jdi jtreg tests on both AIX and Linux x64, I had
some issues but they seemed preexisting (same issues without
my patch). Could you please also run tests you think are
necessary on your platforms? Thanks!
  
  
  best Regards, Thomas


  On Wed, Jun 14, 2017 at 1:36 AM, serguei.spit...@oracle.com

wrote:

  
  On
6/13/17 14:16, serguei.spit...@oracle.com
wrote:
  
  
Hi
  Thomas,
  
  The fix looks great to me.

  
  
 Forgot to say that the copyright comment need an
update.

Thanks,
Serguei
  
  
  

  Thank you for identifying and fixing the problem!
  
  Thanks,
  Serguei
  
  
  On 6/13/17 06:55, Thomas Stüfe wrote:


  Ping... Anyone?

  On Thu, Jun 1, 2017
at 2:18 PM, Thomas Stüfe 
wrote:

  Hi all,


please take a look at this proposed
  fix for a theoretical race in the jdwp
  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
  handling (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
  for 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

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-15 Thread serguei.spit...@oracle.com 

  
  
Hi Thomas,
  
  Sur.
  I'll also run some internal jdi tests.
  
  Thanks,
  Serguei
  
  
  On 6/13/17 21:17, Thomas Stüfe wrote:


  
  Hi Sergey,


thank you for the review! 


I ran jdi jtreg tests on both AIX and Linux x64, I had some
  issues but they seemed preexisting (same issues without my
  patch). Could you please also run tests you think are
  necessary on your platforms? Thanks!


best Regards, Thomas
  
  
On Wed, Jun 14, 2017 at 1:36 AM, serguei.spit...@oracle.com
  
  wrote:
  

On
  6/13/17 14:16, serguei.spit...@oracle.com
  wrote:


  Hi
Thomas,

The fix looks great to me.
  


   Forgot to say that the copyright comment need an
  update.
  
  Thanks,
  Serguei



  
Thank you for identifying and fixing the problem!

Thanks,
Serguei


On 6/13/17 06:55, Thomas Stüfe wrote:
  
  
Ping... Anyone?
  
On Thu, Jun 1, 2017 at
  2:18 PM, Thomas Stüfe 
  wrote:
  
Hi all,
  
  
  please take a look at this proposed
fix for a theoretical race in the jdwp
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
handling (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 for 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

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-14 Thread Thomas Stüfe 
Hi Folks,

may I please have a second reviewer?

Thank you!

Kind Regards, Thomas

On Tue, Jun 13, 2017 at 11:16 PM, serguei.spit...@oracle.com <
serguei.spit...@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üfe wrote:
>
> Ping... Anyone?
>
> On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe 
> wrote:
>
>> Hi all,
>>
>> please take a look at this proposed fix for a theoretical race in the
>> jdwp 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
>> handling (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 for
>> 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
>>
>
>
>

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-13 Thread Thomas Stüfe 
Hi Sergey,

thank you for the review!

I ran jdi jtreg tests on both AIX and Linux x64, I had some issues but they
seemed preexisting (same issues without my patch). Could you please also
run tests you think are necessary on your platforms? Thanks!

best Regards, Thomas

On Wed, Jun 14, 2017 at 1:36 AM, serguei.spit...@oracle.com <
serguei.spit...@oracle.com> wrote:

> On 6/13/17 14:16, serguei.spit...@oracle.com wrote:
>
> Hi Thomas,
>
> The fix looks great to me.
>
>
> Forgot to say that the copyright comment need an update.
>
> Thanks,
> Serguei
>
>
> Thank you for identifying and fixing the problem!
>
> Thanks,
> Serguei
>
>
> On 6/13/17 06:55, Thomas Stüfe wrote:
>
> Ping... Anyone?
>
> On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe 
> wrote:
>
>> Hi all,
>>
>> please take a look at this proposed fix for a theoretical race in the
>> jdwp 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
>> handling (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 for
>> 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
>>
>
>
>
>

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-13 Thread serguei.spit...@oracle.com 

  
  
On 6/13/17 14:16,
  serguei.spit...@oracle.com wrote:


  
  Hi Thomas,

The fix looks great to me.
  


Forgot to say that the copyright comment need an update.

Thanks,
Serguei



   Thank you for identifying and fixing
the problem!

Thanks,
Serguei


On 6/13/17 06:55, Thomas Stüfe wrote:
  
  
Ping... Anyone?
  
On Thu, Jun 1, 2017 at 2:18 PM,
  Thomas Stüfe 
  wrote:
  
Hi all,
  
  
  please take a look at this proposed fix for a
theoretical race in the jdwp 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 handling (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 for 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

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-13 Thread serguei.spit...@oracle.com 

  
  
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üfe wrote:


  
  Ping... Anyone?

  On Thu, Jun 1, 2017 at 2:18 PM,
Thomas Stüfe 
wrote:

  Hi all,


please take a look at this proposed fix for a
  theoretical race in the jdwp 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 handling (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 for 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

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-13 Thread Thomas Stüfe 
Ping... Anyone?

On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe 
wrote:

> Hi all,
>
> please take a look at this proposed fix for a theoretical race in the jdwp
> 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 handling
> (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 for
> 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
>