Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8181419

Race in jdwp invoker handling may lead to crashes or invalid results

    Details

    • Subcomponent:
    • Resolved In Build:
      b13

      Backports

        Description

        (Issue was first found and analyzed by Ralf Schmelter at SAP. Thanks to him for his work.)

        At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

        Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

        The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

        Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

        Pseudocode:

        {code}
        invoker_completeInvokeRequest()
        enter invokerLock;
        A reset Thread's InvokeRequest structure:
                      request->pending = JNI_FALSE;
                      request->started = JNI_FALSE;
                      request->available = JNI_TRUE; /* For next time around */
        retrieve exception ref and return value object ref from InvokeRequest structure
        B leave invokerLock();
        C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
        enter invokerLock;
        D release exception ref and return value object ref *by accessing the thread request structure*
        leave invokerLock();
        {code}

        At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

        At (B) we leave lock protection and from now on a new invoke request may be filled in immediately by the JDWP listener thread.

        At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

        ==========

        IMHO there are several ways to fix this:

        1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be served.

        2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

        3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

        I favour (2) though, because it is a less invasive change.

          Issue Links

            Activity

            stuefe Thomas Stuefe created issue -
            stuefe Thomas Stuefe made changes -
            Field Original Value New Value
            Link This issue relates to JDK-8153711 [ JDK-8153711 ]
            stuefe Thomas Stuefe made changes -
            Link This issue relates to JDK-8154529 [ JDK-8154529 ]
            stuefe Thomas Stuefe made changes -
            Description
            At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            invoker_completeInvokeRequest()

            enter invokerLock;

            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */

            retrieve exception ref and return value object ref from InvokeRequest structure

            B leave invokerLock();



            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.

            enter invokerLock;

            D release exception ref and return value object ref *by accessing the thread request structure*

            leave invokerLock();


            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filed immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be filed.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {noformat}
            invoker_completeInvokeRequest()

            enter invokerLock;

            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */

            retrieve exception ref and return value object ref from InvokeRequest structure

            B leave invokerLock();



            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.

            enter invokerLock;

            D release exception ref and return value object ref *by accessing the thread request structure*

            leave invokerLock();
            {noformat}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filed immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be filed.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            stuefe Thomas Stuefe made changes -
            Description At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {noformat}
            invoker_completeInvokeRequest()

            enter invokerLock;

            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */

            retrieve exception ref and return value object ref from InvokeRequest structure

            B leave invokerLock();



            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.

            enter invokerLock;

            D release exception ref and return value object ref *by accessing the thread request structure*

            leave invokerLock();
            {noformat}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filed immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be filed.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:


            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();


            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filed immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be filed.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            stuefe Thomas Stuefe made changes -
            Description At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:


            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();


            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filed immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be filed.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {{
            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();
            }}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filed immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be filed.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            stuefe Thomas Stuefe made changes -
            Description At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {{
            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();
            }}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filed immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be filed.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {{
            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();
            }}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filled in immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be filed.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            stuefe Thomas Stuefe made changes -
            Description At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {{
            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();
            }}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filled in immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be filed.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {{
            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();
            }}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filled in immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be served.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            stuefe Thomas Stuefe made changes -
            Description At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {{
            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();
            }}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filled in immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be served.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {code}
            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();
            {code}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filled in immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be served.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            gtriantafill George Triantafillou made changes -
            Fix Version/s 10 [ 16302 ]
            gtriantafill George Triantafillou made changes -
            Labels rt-triage-10
            gtriantafill George Triantafillou made changes -
            Status New [ 10000 ] Open [ 1 ]
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk10/jdk10/jdk/rev/b0d1ada042b6
            User: stuefe
            Date: 2017-06-19 10:58:07 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk10/jdk10/jdk/rev/b0d1ada042b6 User: stuefe Date: 2017-06-19 10:58:07 +0000
            hgupdate HG Updates made changes -
            Status Open [ 1 ] Resolved [ 5 ]
            Resolved In Build master [ 18256 ]
            Resolution Fixed [ 1 ]
            stuefe Thomas Stuefe made changes -
            Description At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {code}
            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();
            {code}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filled in immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be served.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            (Issue was first found and analyzed by Ralf Schmelter at SAP. Thanks to him for his work.)

            At the end of JDWP invoke handling we want to release all global references the invoke may have created. This may also include the return value object ref, if return value was an object, and the exception ref, if an exception was thrown.

            Both return value object ref and exception ref need to be kept alive during the sending of the JDWP answer packet, because during sending references are translated into objectIDs and both references have to be not yet collected to do that.

            The sending of the answer packet must happen outside the invokerLock, to minimize the chance for deadlocks.

            Once sending is done, we enter the invokerLock again and clean up return value object ref and exception object ref.

            Pseudocode:

            {code}
            invoker_completeInvokeRequest()
            enter invokerLock;
            A reset Thread's InvokeRequest structure:
                          request->pending = JNI_FALSE;
                          request->started = JNI_FALSE;
                          request->available = JNI_TRUE; /* For next time around */
            retrieve exception ref and return value object ref from InvokeRequest structure
            B leave invokerLock();
            C send answer packet containing exception ref and return value object ref, both will be resolved to objectIDs.
            enter invokerLock;
            D release exception ref and return value object ref *by accessing the thread request structure*
            leave invokerLock();
            {code}

            At (A) we clear the InvokeRequest structure of the thread for the next invoke request.

            At (B) we leave lock protection and from now on a new invoke request may be filled in immediately by the JDWP listener thread.

            At (D) we release the references by taking them from the thread request structure. I think this is wrong, because the request structure may already have been filled by another completed invoke request, whose return values we now delete. If that request answer is sent after ours, it will encounter invalid object references.

            ==========

            IMHO there are several ways to fix this:

            1) move the "request->available = JNI_TRUE;" down after the answer packet was sent. But this opens the possibility for deadlocks, because while this (potentially longish) IO happens, no other invoke request can be served.

            2) We do not need to access the thread request structure to release the object references contained in it. Just use local variables to keep the object references to release.

            3) We also could split outStream_writeObjectRef() into two parts, one which gets the objectID, one which writes the objectID. Then, we could get the objectIDs for exception ref and return value object ref before (B), release them also before (B), and just send back the objectIDs in C.

            I favour (2) though, because it is a less invasive change.
            hgupdate HG Updates made changes -
            Resolved In Build master [ 18256 ] b13 [ 17339 ]
            Hide
            sgehwolf Severin Gehwolf added a comment -
            Fix Request as per http://openjdk.java.net/projects/jdk9/fix-request-process

            This bug is an improvement to JDK-8153711 which is already in JDK 9. Thus, it would make sense to also get this bug into JDK 9 as it only makes the fix of JDK-8153711 more robust. Test coverage is provided by the test added with JDK-8153711, test/com/sun/jdi/OomDebugTest.java. I'm the author of the original fix JDK-8153711 and have reviewed this fix. Serguei Spitsyn reviewed it too. He was one of the reviewers of the original fix.

            Patch for JDK 10 applies as is for JDK 9, it builds fine for me and passes the test.
            Show
            sgehwolf Severin Gehwolf added a comment - Fix Request as per http://openjdk.java.net/projects/jdk9/fix-request-process This bug is an improvement to JDK-8153711 which is already in JDK 9. Thus, it would make sense to also get this bug into JDK 9 as it only makes the fix of JDK-8153711 more robust. Test coverage is provided by the test added with JDK-8153711 , test/com/sun/jdi/OomDebugTest.java. I'm the author of the original fix JDK-8153711 and have reviewed this fix. Serguei Spitsyn reviewed it too. He was one of the reviewers of the original fix. Patch for JDK 10 applies as is for JDK 9, it builds fine for me and passes the test.
            sgehwolf Severin Gehwolf made changes -
            Labels rt-triage-10 jdk9-fix-request rt-triage-10
            sgehwolf Severin Gehwolf made changes -
            Affects Version/s 8u152 [ 18805 ]
            sgehwolf Severin Gehwolf made changes -
            Link This issue relates to JDK-8186031 [ JDK-8186031 ]
            jwilhelm Jesper Wilhelmsson made changes -
            Link This issue backported by JDK-8186031 [ JDK-8186031 ]
            jwilhelm Jesper Wilhelmsson made changes -
            Link This issue relates to JDK-8186031 [ JDK-8186031 ]
            hgupdate HG Updates made changes -
            Link This issue backported by JDK-8186555 [ JDK-8186555 ]
            hgupdate HG Updates made changes -
            Link This issue backported by JDK-8188624 [ JDK-8188624 ]

              People

              • Assignee:
                stuefe Thomas Stuefe
                Reporter:
                stuefe Thomas Stuefe
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: