Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebSocket: Close #195

Closed
EricForgy opened this issue Feb 12, 2018 · 4 comments
Closed

WebSocket: Close #195

EricForgy opened this issue Feb 12, 2018 · 4 comments

Comments

@EricForgy
Copy link
Contributor

HI,

I'm struggling a little bit with closewrite. If I understand, it is kind of a "partial" close that toggles the txclosed indicator, sends a CLOSE opcode, but leaves the underlying socket open. However, if I look at the WebSockets spec:

https://tools.ietf.org/html/rfc6455#section-5.5.1

   The application MUST NOT send any more data frames after sending a
   Close frame.

   If an endpoint receives a Close frame and did not previously send a
   Close frame, the endpoint MUST send a Close frame in response.  (When
   sending a Close frame in response, the endpoint typically echos the
   status code it received.)  It SHOULD do so as soon as practical.  An
   endpoint MAY delay sending a Close frame until its current message is
   sent (for instance, if the majority of a fragmented message is
   already sent, an endpoint MAY send the remaining fragments before
   sending a Close frame).  However, there is no guarantee that the
   endpoint that has already sent a Close frame will continue to process
   data.

   After both sending and receiving a Close message, an endpoint
   considers the WebSocket connection closed and MUST close the
   underlying TCP connection.  The server MUST close the underlying TCP
   connection immediately; the client SHOULD wait for the server to
   close the connection but MAY close the connection at any time after
   sending and receiving a Close message, e.g., if it has not received a
   TCP Close from the server in a reasonable time period.

Does this mean we should NOT have a closewrite for WebSockets? If a CLOSE opcode is sent, we should not be sending any addition data over the websocket.

I'm playing around with a branch that uses ReadyState borrowed from the original WebSockets.jl package so txclosed becomes txstate and rxclosed becomes rxstate, but given the above, I am wondering if we need to keep track of tx and rx states, Maybe it should just be a single state.

What do you think?

@EricForgy
Copy link
Contributor Author

I submitted a PR ( #197 ) that would make this issue moot 🙏

@samoconnor
Copy link
Contributor

I'm not in favour of switching to a different implementation without understanding the root cause.

closewrite means "I don't want to send any more data and I want the other end to know that".

closewrite(::WebSocket) sends a close frame and sets a flag to protect against double-closing.

close means "I don' want to send or receive any more".

Base.close(::WebSocket) calls closewrite to send the close frame if needed, then calls readframe until a close frame is received from the other end.

readframe sets the ws.rxclosed flag when if reads a close frame, this causes the loop in close to terminate.

The close function should (but does not) call close(ws.io) like this:

function Base.close(ws::WebSocket)
    if !ws.txclosed
        closewrite(ws)
    end
    while !ws.rxclosed
        readframe(ws)
    end
    close(ws.io)
end

The application MUST NOT send any more data frames after sending a Close frame.

That's fine. It just means that users should not write anything to the socket after they call closewrite. If they never call closewrite they don't have to care about this. It also means that users should not write anything after calling close because close calls closewrite, but not writing after close is kind of obvious.

If an endpoint receives a Close frame and did not previously send a Close frame, the endpoint MUST send a Close frame in response. ... An endpoint MAY delay sending a Close frame until its current message is sent

So users should call close after a close frame and after they are done sending the last thing they want to send. At present the user would have to call isopen to check if the close frame was received, then call close. It would probably make sense to modify eof to detect the close frame also:

Base.eof(ws::WebSocket) = ws.rxclosed || eof(ws.io)

That way users could do:

while !eof(ws)                # ws.rxclosed checked here
    x = readavailable(ws)     # ws.rxclosed set here when close frame is recieved
end
close(ws)                     # close frame is sent here when close calls closewrite

After both sending and receiving a Close message, an endpoint considers the WebSocket connection closed and MUST close the underlying TCP connection.

This is missing (see note above about adding close(ws.io) to close(::WebSocket)).

@samoconnor
Copy link
Contributor

I've merged your tests into this commit: 2cea2f7

@EricForgy
Copy link
Contributor Author

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants