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

WSLPeer::put_packet has an unbound outgoing buffer. Websocket outgoing buffers unused. #50988

Closed
jordo opened this issue Jul 28, 2021 · 2 comments · Fixed by #51036
Closed

WSLPeer::put_packet has an unbound outgoing buffer. Websocket outgoing buffers unused. #50988

jordo opened this issue Jul 28, 2021 · 2 comments · Fixed by #51036

Comments

@jordo
Copy link
Contributor

jordo commented Jul 28, 2021

Godot version

3.X, 4.X

System information

All

Issue description

WebsocketPeer, when running through the WSLPeer implementation (anything non HTML5), has an unbound output message buffer.

This manifests itself under conditions where outbound traffic on the put_packet() interface under WebSocketPeer exceeds what the far-end can pull out.

The output buffer configuration in WSLPeer::make_context() is unused (and actually the p_out_pkt_size parameter is incorrectly being used to configure the receiving _packet_buffer size) . This causes the underlying wslay wslay_event_queue_msg() to indefinitely queue up messages into the internal wslay message queue. This queue is unbound.

Steps to reproduce

Send traffic through WSLPeer::put_packet() and observe the internal wslay queue length in wslay_queue_push().

Minimal reproduction project

No response

@Calinou Calinou added this to the 3.4 milestone Jul 28, 2021
@jordo
Copy link
Contributor Author

jordo commented Jul 28, 2021

This impacts the following buffer settings. They appear to be unused:

#define WSC_OUT_BUF "network/limits/websocket_client/max_out_buffer_kb"
#define WSC_OUT_PKT "network/limits/websocket_client/max_out_packets"

#define WSS_OUT_BUF "network/limits/websocket_server/max_out_buffer_kb"
#define WSS_OUT_PKT "network/limits/websocket_server/max_out_packets"

@jordo jordo changed the title WSLPeer::put_packet has an unbound outgoing buffer WSLPeer::put_packet has an unbound outgoing buffer. Websocket outgoing bufferes unused. Jul 28, 2021
@jordo jordo changed the title WSLPeer::put_packet has an unbound outgoing buffer. Websocket outgoing bufferes unused. WSLPeer::put_packet has an unbound outgoing buffer. Websocket outgoing buffers unused. Jul 28, 2021
@jordo
Copy link
Contributor Author

jordo commented Jul 28, 2021

@Fales the likely fix here is to query wslay_event_get_queued_msg_count() and wslay_event_get_queued_msg_length() prior to sending a packet to wslay_event_queue_msg().

But I want to propose exposing these two buffer queue query sizes as well, and I'll explain my reasoning below.

WebsocketPeer is effectively a packet peer (implements a packet peer api) in Godot, but is backed by a stream (tcp). So it's important to be able to query the current outbound buffer capacity before stuffing it. We end up using the WebsocketPeer for a variety of use cases in our application, but I would love to inspect the current size of the outbound buffer, because our application could make better decisions about utilization with the ability to inspect the current outbound buffer.

The WebsocketPeer buffers in godot, are application level and configurable, so it would be great to expose the ability to inspect the current status of each buffer. It allows more flexibility for the application to decide what to do when approaching buffer limits.

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

Successfully merging a pull request may close this issue.

3 participants