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

Socket should flush before closing #405

Open
TroyKomodo opened this issue Jan 31, 2024 · 1 comment
Open

Socket should flush before closing #405

TroyKomodo opened this issue Jan 31, 2024 · 1 comment

Comments

@TroyKomodo
Copy link

I noticed some strange behavior in the way closes are handled when we are the server and a client initiates a close.

The server responds with a close frame, however this close frame is not sent unless we call flush one more time on the socket.
I think it should be the library's responsibility to flush the socket when a close frame is sent internally by the library.

No flush call

2024-01-31T18:20:16.082142Z TRACE tungstenite::protocol::frame::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/frame.rs:144: Parsed headers [136, 130]    
2024-01-31T18:20:16.082154Z TRACE tungstenite::protocol::frame::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/frame.rs:148: First: 10001000    
2024-01-31T18:20:16.082162Z TRACE tungstenite::protocol::frame::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/frame.rs:149: Second: 10000010    
2024-01-31T18:20:16.082172Z TRACE tungstenite::protocol::frame::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/frame.rs:158: Opcode: Control(Close)    
2024-01-31T18:20:16.082182Z TRACE tungstenite::protocol::frame::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/frame.rs:161: Masked: true    
2024-01-31T18:20:16.082192Z TRACE tungstenite::protocol::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/mod.rs:200: received frame 
<FRAME>
final: true
reserved: false false false
opcode: CLOSE
length: 8
payload length: 2
payload: 0x5ce3
                
2024-01-31T18:20:16.082215Z DEBUG tungstenite::protocol: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/mod.rs:675: Received close frame: Some(CloseFrame { code: Normal, reason: "" })    
2024-01-31T18:20:16.082227Z DEBUG tungstenite::protocol: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/mod.rs:692: Replying to close with Frame { header: FrameHeader { is_final: true, rsv1: false, rsv2: false, rsv3: false, opcode: Control(Close), mask: None }, payload: [3, 232] }    
2024-01-31T18:20:16.082239Z TRACE tungstenite::protocol: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/mod.rs:413: Received message     
image

Explicit flush call

2024-01-31T18:18:02.272064Z TRACE tungstenite::protocol::frame::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/frame.rs:144: Parsed headers [136, 130]    
2024-01-31T18:18:02.272073Z TRACE tungstenite::protocol::frame::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/frame.rs:148: First: 10001000    
2024-01-31T18:18:02.272081Z TRACE tungstenite::protocol::frame::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/frame.rs:149: Second: 10000010    
2024-01-31T18:18:02.272090Z TRACE tungstenite::protocol::frame::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/frame.rs:158: Opcode: Control(Close)    
2024-01-31T18:18:02.272100Z TRACE tungstenite::protocol::frame::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/frame.rs:161: Masked: true    
2024-01-31T18:18:02.272109Z TRACE tungstenite::protocol::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/mod.rs:200: received frame 
<FRAME>
final: true
reserved: false false false
opcode: CLOSE
length: 8
payload length: 2
payload: 0x9231
                
2024-01-31T18:18:02.272132Z DEBUG tungstenite::protocol: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/mod.rs:675: Received close frame: Some(CloseFrame { code: Normal, reason: "" })    
2024-01-31T18:18:02.272144Z DEBUG tungstenite::protocol: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/mod.rs:692: Replying to close with Frame { header: FrameHeader { is_final: true, rsv1: false, rsv2: false, rsv3: false, opcode: Control(Close), mask: None }, payload: [3, 232] }    
2024-01-31T18:18:02.272155Z TRACE tungstenite::protocol: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/mod.rs:413: Received message     
2024-01-31T18:18:02.272201Z TRACE tungstenite::protocol: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/mod.rs:494: Sending pong/close    
2024-01-31T18:18:02.272210Z TRACE tungstenite::protocol: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/mod.rs:724: Sending frame: Frame { header: FrameHeader { is_final: true, rsv1: false, rsv2: false, rsv3: false, opcode: Control(Close), mask: None }, payload: [3, 232] }    
2024-01-31T18:18:02.272220Z TRACE tungstenite::protocol::frame: /home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tungstenite-0.21.0/src/protocol/frame/mod.rs:219: writing frame 
<FRAME>
final: true
reserved: false false false
opcode: CLOSE
length: 4
payload length: 2
payload: 0x03e8
image
TroyKomodo added a commit to SevenTV/SevenTV that referenced this issue Jan 31, 2024
Fix sending a close frame on websocket disconnect, bug was found due to
dankchat's websocket library being very strict with closing frames.
Upstream bug report
snapview/tungstenite-rs#405

Tungstenite does not flush the frame buffer on close, so when it echo's
back a close to the client it does not send that close unless we
explicitly call flush on the buffer.
@daniel-abramov
Copy link
Member

daniel-abramov commented Jan 31, 2024

If I got you right, it sounds like a bug in a user code (incorrect usage) rather than in a library, because AFAIR the Autobahn suite does have a test that specifically checks this behavior. Autobahn tests are part of our CI pipeline.

There is one specific usage scenario that would result in the behavior that you describe:

  • The user reads a message via tungstenite.
  • Sees that it's a close message.
  • Stops reading/sending any messages.
  • States that tungstenite did not send the response to the close frame :)

If this is the case, you must ensure that you're reading frames until the WebSocket connection is closed, not until you receive a close frame, otherwise, you don't give tungstenite a chance to flush the buffer.

If this is not the case, could you provide a simple example that demonstrates/reproduces the bug?

TroyKomodo added a commit to SevenTV/SevenTV that referenced this issue Feb 2, 2024
Implements the event-api from https://github.com/SevenTV/eventapi in Rust. This implementation improves
memory usage by 8x and CPU usage by 4x. Taking bytes per connection from around 300kb -> 40kb and CPU
usage from 40 cores down to 10. This aims to be a 1:1 implementation of the existing event-api with
improvements and optimizations around memory and cpu usage. With very few minor changes from the
initial implementation.

Notably:
- Subscriptions cannot have a TTL.
- Subscriptions which were automatically subscribed to get unsubscribed after a few minutes
- Hash Deduping does not dedupe for the entire lifetime of the connection, but rather for the next 255 events.
- Path subscriptions must be URL encoded values.
- Added header subscriptions, (JSON array of Subscriptions).
- Path subscriptions work on WebSockets.
- Added query parameter subscription, (JSON array of Subscriptions).

* bug: websocket close frame

Fix sending a close frame on websocket disconnect, bug was found due to
dankchat's websocket library being very strict with closing frames.
Upstream bug report
snapview/tungstenite-rs#405

Tungstenite does not flush the frame buffer on close, so when it echo's
back a close to the client it does not send that close unless we
explicitly call flush on the buffer.
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