# HG changeset patch # User stuefe # Date 1497865921 -7200 # Mon Jun 19 11:52:01 2017 +0200 # Node ID ca0c7b2783e0102468218589a062e7ac4736aae2 # Parent 148a7d6c463ad1726bad8a9e8d5df191314d704b 8181419, PR3413, RH1463144: Race in jdwp invoker handling may lead to crashes or invalid results Reviewed-by: sspitsyn, sgehwolf, clanger diff --git a/src/share/back/invoker.c b/src/share/back/invoker.c --- openjdk/jdk/src/share/back/invoker.c +++ openjdk/jdk/src/share/back/invoker.c @@ -212,30 +212,6 @@ } /* - * Delete saved global references - if any - for: - * - a potentially thrown Exception - * - a returned refernce/array value - * See invoker_doInvoke() and invoke* methods where global references - * are being saved. - */ -static void -deletePotentiallySavedGlobalRefs(JNIEnv *env, InvokeRequest *request) -{ - /* Delete potentially saved return value */ - if ((request->invokeType == INVOKE_CONSTRUCTOR) || - (returnTypeTag(request->methodSignature) == JDWP_TAG(OBJECT)) || - (returnTypeTag(request->methodSignature) == JDWP_TAG(ARRAY))) { - if (request->returnValue.l != NULL) { - tossGlobalRef(env, &(request->returnValue.l)); - } - } - /* Delete potentially saved exception */ - if (request->exception != NULL) { - tossGlobalRef(env, &(request->exception)); - } -} - -/* * Delete global argument references from the request which got put there before a * invoke request was carried out. See fillInvokeRequest(). */ @@ -744,6 +720,7 @@ jint id; InvokeRequest *request; jboolean detached; + jboolean mustReleaseReturnValue = JNI_FALSE; JDI_ASSERT(thread); @@ -787,6 +764,13 @@ id = request->id; exc = request->exception; returnValue = request->returnValue; + + /* Release return value and exception references, but delay the release + * until after the return packet was sent. */ + mustReleaseReturnValue = request->invokeType == INVOKE_CONSTRUCTOR || + returnTypeTag(request->methodSignature) == JDWP_TAG(OBJECT) || + returnTypeTag(request->methodSignature) == JDWP_TAG(ARRAY); + } /* @@ -801,6 +785,12 @@ */ deleteGlobalArgumentRefs(env, request); + /* From now on, do not access the request structure anymore + * for this request id, because once we give up the invokerLock it may + * be immediately reused by a new invoke request. + */ + request = NULL; + /* * Give up the lock before I/O operation */ @@ -821,7 +811,12 @@ */ eventHandler_lock(); // for proper lock order debugMonitorEnter(invokerLock); - deletePotentiallySavedGlobalRefs(env, request); + if (mustReleaseReturnValue && returnValue.l != NULL) { + tossGlobalRef(env, &returnValue.l); + } + if (exc != NULL) { + tossGlobalRef(env, &exc); + } debugMonitorExit(invokerLock); eventHandler_unlock(); }