Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-849

gremlin-server doesn't close sessions on 'close' opcode

    Details

      Description

      When sending a packet with a close opcode (0x8). One would expect the server to terminate the socket and all sessions created through it.

      Currently this is not the case and this can be tested by doing the following:

      • Open websocket (handshake etc..)
      • Send a message with a session UUID (that we'll call x) and a script : cal = 5+5
      • Send a 'close' request and terminate the socket.
      • Open another websocket connection
      • Send a message with a session UUID of x and a script : cal

      This last event should generate an error (cal shouldn't be defined). Instead it continues by sending back 10 as if the session were still alive.

      -------------------------
      Alternatively:
      If there is no tracking of the sessions opened through a specific socket connection, then perhaps we can join the following payload to the 'close' packet:

      { "requestId":"1d6d02bd-8e56-421d-9438-3bd6d0079ff1",
        "op":"eval",
        "processor":"session",
        "args":{"gremlin":"",
                "session":"x"}
      }
      

      The server will know to terminate session x at this stage.

        Activity

        Hide
        dmill Dylan Millikin added a comment -

        Ok great this works as expected, tests pass now. Thanks a lot.
        (I was confusing an error I was getting from the lack of db response with an issue in the functionality)

        Show
        dmill Dylan Millikin added a comment - Ok great this works as expected, tests pass now. Thanks a lot. (I was confusing an error I was getting from the lack of db response with an issue in the functionality)
        Hide
        spmallette stephen mallette added a comment -

        The "op" key should be set to the value of "close". I think "args" should just need a "session" key with the session id. Follow that request with your websocket close op.

        Show
        spmallette stephen mallette added a comment - The "op" key should be set to the value of "close". I think "args" should just need a "session" key with the session id. Follow that request with your websocket close op.
        Hide
        dmill Dylan Millikin added a comment -

        Thanks Stephen. Just to make sure I understand. Should I send one close header with a payload like the one I posted? Then close the socket? I gave this (and many other variations) a try without success, I'm pretty sure I'm miss-understanding something.

        "close" op is a little ambiguous as it can refer to both the packet header and the payload operation. That's in part why I'm a little confused.

        Thanks for clearing that out.

        Show
        dmill Dylan Millikin added a comment - Thanks Stephen. Just to make sure I understand. Should I send one close header with a payload like the one I posted? Then close the socket? I gave this (and many other variations) a try without success, I'm pretty sure I'm miss-understanding something. "close" op is a little ambiguous as it can refer to both the packet header and the payload operation. That's in part why I'm a little confused. Thanks for clearing that out.
        Hide
        spmallette stephen mallette added a comment -

        I noticed this the other day (as a result of the issue you were noticing with titan+berkeley) and was kinda surprised that this was another instance where we didn't have a clean close of the session. Anyway, I've made some changes in tp30/master to allow you to send a "close" op code with a request message that uses the "session" processor. That "close" op will rollback any open transactions. The "close" op code should still be followed by a websocket close frame to client-side kill the websocket itself.

        I thought about trying to allow one to embed the payload directly into the close frame, but that some problems in that i wanted the subprotocol to be protocol agnostic, so it should work with or without knowledge of websockets. I kinda found a way around that which was workable, but then i hit problems with the serialization model which expected all requests with payload to be either text or binary framed and so if the close frame included a payload, which serializer would it use?

        In the end I just left it as having to send two separate frames to fully close. In either case, I would expect the server to clean up if one message was sent but not the other. There would just be some dangling resources hanging around until that happened.

        Show
        spmallette stephen mallette added a comment - I noticed this the other day (as a result of the issue you were noticing with titan+berkeley) and was kinda surprised that this was another instance where we didn't have a clean close of the session. Anyway, I've made some changes in tp30/master to allow you to send a "close" op code with a request message that uses the "session" processor. That "close" op will rollback any open transactions. The "close" op code should still be followed by a websocket close frame to client-side kill the websocket itself. I thought about trying to allow one to embed the payload directly into the close frame, but that some problems in that i wanted the subprotocol to be protocol agnostic, so it should work with or without knowledge of websockets. I kinda found a way around that which was workable, but then i hit problems with the serialization model which expected all requests with payload to be either text or binary framed and so if the close frame included a payload, which serializer would it use? In the end I just left it as having to send two separate frames to fully close. In either case, I would expect the server to clean up if one message was sent but not the other. There would just be some dangling resources hanging around until that happened.

          People

          • Assignee:
            spmallette stephen mallette
            Reporter:
            dmill Dylan Millikin
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development