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

[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-bounces at openjdk.java.net] On Behalf Of Thomas Stüfe
Sent: Donnerstag, 15. Juni 2017 08:31
To: serviceability-dev at 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 at oracle.com<mailto:serguei.spitsyn at oracle.com> <serguei.spitsyn at oracle.com<mailto:serguei.spitsyn at 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 <thomas.stuefe at gmail.com<mailto:thomas.stuefe at 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



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20170619/d0f6a0c3/attachment-0001.html>