Possible race in jdwp invoke handling?

[serguei.spitsyn at oracle.com]

Hi Thomas,

I'll look at it a couple of days later as I'm busy with other stuff now.

Thanks,
Serguei


On 5/31/17 07:42, Thomas Stüfe wrote:
> Hi all,
>
> I am looking at a possible race in JDWP invoke request handling and 
> would like your opinion.
>
> This is how I understand the handling of invoke events (please do 
> correct me if I am wrong):
>
> 1) JDWP InvokeXXX request comes in for thread X. Handled by the "JDWP 
> Transport Listener" thread. We call "invoker_requestInvoke()". Here, 
> under lock protection, we take the thread-local InvokeRequest 
> structure and fill it with the invoke data. We only do this if 
> request->available is true. We set request->available to false, 
> thereby preventing further attempts to add invoke requests for this 
> thread. Any subsequent JDWP invoke commands will now return with 
> errors, right?
>
> 2) In the context of a JVMTI callback for thread X the actual invoke 
> will be done. Request structure will be filled with the result 
> (exception object handle and result). request->available stays false.
>
> 3) In a third thread, the "JDWP Event Helper Thread", the return 
> packet will be sent to the debugger. In 
> invoker_completeInvokeRequest(), we have two guarded sections. In the 
> first section, we reset request->available to true (A):
>
>     eventHandler_lock(); /* for proper lock order */
>     debugMonitorEnter(invokerLock);
>
>     request = threadControl_getInvokeRequest(thread);
>     ....<skip>...
>
>     request->pending = JNI_FALSE;
>     request->started = JNI_FALSE;
> A) request->available = JNI_TRUE; /* For next time around */
>
>     ...<skip>...
>
>     debugMonitorExit(invokerLock);
>     eventHandler_unlock();
>
> Then we leave the guarded section and send the jdwp answer packet back.
>
> Then we enter a second guarded section and clean up the handles for 
> return value and exception:
>
> ...
>     eventHandler_lock(); // for proper lock order
>     debugMonitorEnter(invokerLock);
> B)    deletePotentiallySavedGlobalRefs(env, request);
>     debugMonitorExit(invokerLock);
>     eventHandler_unlock();
>
>
> ---
>
> My question is this: would it be possible for a new invoke request to 
> be incoming in the time between the first and the second guarded 
> section? So, could the following sequence happen:
>
>
> [JDWP Transport Listener]         invoker_requestInvoke (request 1)
>
> [Thread X]  invoker_doInvoke  (request 1)
>
> [JDWP Event Helper]  invoker_completeInvokeRequest  (request 1)
> debugMonitorEnter(invokerLock);
> <reset request->available and request->pending>
> debugMonitorExit(invokerLock);
> ....
>
> [JDWP Transport Listener]             invoker_requestInvoke (request 2)
>
> [Thread X]  invoker_doInvoke  (request 2)  -> overwrites 
> request->exception and request->returnValue to its new values.
>
>
>
> [JDWP Event Helper]                     ....
>  debugMonitorEnter(invokerLock);
>  deletePotentiallySavedGlobalRefs(env, request);  -> releases 
> request->exception and request->returnValue, which is now interfering 
> with request 2.
>  debugMonitorExit(invokerLock);
>
>
> ?
>
> This is all theoretical. Wanted to hear your opinions first before 
> proceeding.
>
> Kind Regards, Thomas
>
>
>
>
>
>