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

[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 <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/
> > 
> > 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