[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Possible race in jdwp invoke handling?
From:       Thomas_Stüfe <thomas.stuefe () gmail ! com>
Date:       2017-05-31 14:42:44
Message-ID: CAA-vtUxoZN+7cH6csKiP8neOJun7PfgE2bJpO78d+HScWV3bVw () mail ! gmail ! com
[Download RAW message or body]

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

[Attachment #3 (text/html)]

<div dir="ltr">Hi all,<div><br></div><div>I am looking at a possible race in JDWP \
invoke request handling and would like your opinion.</div><div><br></div><div>This is \
how I understand the handling of invoke events (please do correct me if I am \
wrong):<br></div><div><br></div><div>1) JDWP InvokeXXX request comes in for thread X. \
Handled by the &quot;JDWP Transport Listener&quot; thread. We call \
&quot;invoker_requestInvoke()&quot;. Here, under lock protection, we take the \
thread-local InvokeRequest structure and fill it with the invoke data. We only do \
this if request-&gt;available is true. We set request-&gt;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?</div><div><br></div><div>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-&gt;available stays false.</div><div><br></div><div>3) In a third \
thread, the &quot;JDWP Event Helper Thread&quot;, the return packet will be sent to \
the debugger. In invoker_completeInvokeRequest(), we have two guarded sections. In \
the first section, we reset request-&gt;available to true \
(A):</div><div><br></div><div><div>      eventHandler_lock(); /* for proper lock \
order */</div><div>      debugMonitorEnter(invokerLock);</div><div><br></div><div>    \
request = threadControl_getInvokeRequest(thread);</div><div>      \
....&lt;skip&gt;...</div><div><br></div><div>      request-&gt;pending = \
JNI_FALSE;</div><div>      request-&gt;started = JNI_FALSE;</div><div>A)  \
request-&gt;available = JNI_TRUE; /* For next time around \
*/</div><div><br></div><div>      ...&lt;skip&gt;...</div><div><br></div><div>      \
debugMonitorExit(invokerLock);<br></div><div>      \
eventHandler_unlock();</div></div><div><br></div><div>     </div><div>Then we leave \
the guarded section and send the jdwp answer packet \
back.</div><div><br></div><div>Then we enter a second guarded section and clean up \
the handles for return value and \
exception:</div><div><br></div><div>...</div><div><div>      eventHandler_lock(); // \
for proper lock order</div><div>      debugMonitorEnter(invokerLock);</div><div>B)    \
deletePotentiallySavedGlobalRefs(env, request);</div><div>      \
debugMonitorExit(invokerLock);</div><div>      \
eventHandler_unlock();</div></div><div><br></div><div><br></div><div>---</div><div><br></div><div>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:</div><div><br></div><div><br></div><div>[JDWP Transport Listener]    \
invoker_requestInvoke (request 1)</div><div><br></div><div>[Thread X]                 \
invoker_doInvoke   (request 1)</div><div><br></div><div>[JDWP Event Helper]           \
invoker_completeInvokeRequest   (request 1)</div><div>                                \
debugMonitorEnter(invokerLock);</div><div>                                            \
&lt;reset request-&gt;available and request-&gt;pending&gt;</div><div>                \
debugMonitorExit(invokerLock);</div><div>                                             \
....</div><div><br></div><div>[JDWP Transport Listener]                   \
invoker_requestInvoke (request 2)<br></div><div><br></div><div>[Thread X]             \
invoker_doInvoke   (request 2)   -&gt; overwrites request-&gt;exception and \
request-&gt;returnValue to its new \
values.<br></div><div><br></div><div><br></div><div><br></div><div>[JDWP Event \
Helper]                               ....</div><div>                                 \
debugMonitorEnter(invokerLock);<br></div><div>                                        \
deletePotentiallySavedGlobalRefs(env, request);   -&gt; releases \
request-&gt;exception and request-&gt;returnValue, which is now interfering with \
request 2.</div><div>                                                                 \
debugMonitorExit(invokerLock);</div><div><br></div><div><br></div><div>?</div><div><br></div><div>This \
is all theoretical. Wanted to hear your opinions first before \
proceeding.</div><div><br></div><div>Kind Regards, \
Thomas</div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div></div>




[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic