tungstenite-rs icon indicating copy to clipboard operation
tungstenite-rs copied to clipboard

Get stream on handshake error

Open canislupaster opened this issue 5 years ago • 6 comments

I'm using mio and tungstenite, and it would be nice to have a way to discard the handle when being refused a handshake:

match mid.handshake() {
	Ok(x) => *space = SocketSpace::Connected(x),
    Err(HandshakeError::Interrupted(mid)) => *space = SocketSpace::Connecting(mid),
    Err(HandshakeError::Failure(err)) => {
       discard(mid.stream?! halp)
    }
}

canislupaster avatar Dec 15 '18 20:12 canislupaster

I think it's possible, but it would require some changes in tungstenite-rs. If I got your request right, there is a change in HandshakeRole required, so that it returns not just an error when something fails, but something like (Error, HandshakeRole::InternalStream). Was that the thing you needed?

NB: If you just need to drop the stream, it will be dropped anyways, as mid.handshake() takes self by move.

daniel-abramov avatar Aug 08 '19 13:08 daniel-abramov

Hi!

What is the state of this Issue? Is there any idea in mind to return a HandshakeError::Failure(Error, HandshakeRole::InternalStream)?

I'm in a situation where I need to deregister the stream from the poll registry if the handshake fails. Since the HandshakeError::Failure is not returning the stream, I'm not able to deregister it. Any workaround?

Thanks!

lemunozm avatar May 09 '21 13:05 lemunozm

What is the state of this Issue?

I have not yet got acknowledgement from the reporter if the behavior that I described in the previous comment is the one that he would expected. There has been no progress on this issue so far.

I'm in a situation where I need to deregister the stream from the poll registry if the handshake fails. Since the HandshakeError::Failure is not returning the stream, I'm not able to deregister it. Any workaround?

So you're using it in conjunction with mio? Won't it get unregistered once the socket is dropped? I'm asking because we actually do provide tokio-tungstenite which relies on tungstenite-rs as its core and it seems to work fine (tokio uses mio under the hood).

daniel-abramov avatar May 10 '21 09:05 daniel-abramov

Thanks for the notification. Yeah, I'm using it with mio.

Until I know from the mio's documentation, it's on the side of the user to call deregister(). If you remove the TcpStream without deregistering it you will leave the entry of the file descriptor of the socket into the OS poll, more or less like a "leak memory".

~~I took a look into the tokio's TcpStream and it doesn't seem that it calls the deregister() when it drops.~~ Neither the mio's TcpStream.

EDIT: I took a deeper look and I can confirm that a tokio's TcpStream is deregistering when it drops (concretely, the PollEvented entity do that job). Although this works for the tokio's one, anyone who can use other TcpStream as mio, will get the "leak".

lemunozm avatar May 10 '21 11:05 lemunozm

Any workaround?

Based on your comments, the only [temporary] dirty hack that comes to my mind is to wrap the TcpStream into another structure, implement Drop (or any other trait if required) and pass it to tungstenite-rs as tungstenite-rs is type agnostic (it just requires the stream to implement Read and Write).

daniel-abramov avatar May 10 '21 14:05 daniel-abramov

If the input stream is not tcpstream but other layers based on it. We may hard to use Drop.

Can you provide another callback for such error? So developer can use this callback to customize error response.

leptonyu avatar Oct 11 '22 04:10 leptonyu