From 0e528676683a0df96751fd0daa018e0818baa0e2 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Sun, 27 Mar 2022 15:30:17 +0200 Subject: [PATCH 1/2] [Net] Make StreamPeerTCP::_poll_connection explicit. No longer hacked into `get_status` and renamed to `poll`. The old `poll` (for *nix `poll`, win `select`) is now called `wait`. --- core/debugger/remote_debugger_peer.cpp | 6 +- core/io/http_client_tcp.cpp | 3 + core/io/stream_peer_tcp.cpp | 76 +++++++++----------------- core/io/stream_peer_tcp.h | 10 ++-- doc/classes/StreamPeerTCP.xml | 8 ++- modules/websocket/wsl_client.cpp | 1 + 6 files changed, 46 insertions(+), 58 deletions(-) diff --git a/core/debugger/remote_debugger_peer.cpp b/core/debugger/remote_debugger_peer.cpp index 7c7d38ab0a8..90c396c2d90 100644 --- a/core/debugger/remote_debugger_peer.cpp +++ b/core/debugger/remote_debugger_peer.cpp @@ -93,7 +93,7 @@ RemoteDebuggerPeerTCP::~RemoteDebuggerPeerTCP() { } void RemoteDebuggerPeerTCP::_write_out() { - while (tcp_client->poll(NetSocket::POLL_TYPE_OUT) == OK) { + while (tcp_client->get_status() == StreamPeerTCP::STATUS_CONNECTED && tcp_client->wait(NetSocket::POLL_TYPE_OUT) == OK) { uint8_t *buf = out_buf.ptrw(); if (out_left <= 0) { if (out_queue.size() == 0) { @@ -119,7 +119,7 @@ void RemoteDebuggerPeerTCP::_write_out() { } void RemoteDebuggerPeerTCP::_read_in() { - while (tcp_client->poll(NetSocket::POLL_TYPE_IN) == OK) { + while (tcp_client->get_status() == StreamPeerTCP::STATUS_CONNECTED && tcp_client->wait(NetSocket::POLL_TYPE_IN) == OK) { uint8_t *buf = in_buf.ptrw(); if (in_left <= 0) { if (in_queue.size() > max_queued_messages) { @@ -167,6 +167,7 @@ Error RemoteDebuggerPeerTCP::connect_to_host(const String &p_host, uint16_t p_po tcp_client->connect_to_host(ip, port); for (int i = 0; i < tries; i++) { + tcp_client->poll(); if (tcp_client->get_status() == StreamPeerTCP::STATUS_CONNECTED) { print_verbose("Remote Debugger: Connected!"); break; @@ -213,6 +214,7 @@ void RemoteDebuggerPeerTCP::poll() { } void RemoteDebuggerPeerTCP::_poll() { + tcp_client->poll(); if (connected) { _write_out(); _read_in(); diff --git a/core/io/http_client_tcp.cpp b/core/io/http_client_tcp.cpp index f9207996774..d983d86b993 100644 --- a/core/io/http_client_tcp.cpp +++ b/core/io/http_client_tcp.cpp @@ -264,6 +264,9 @@ void HTTPClientTCP::close() { } Error HTTPClientTCP::poll() { + if (tcp_connection.is_valid()) { + tcp_connection->poll(); + } switch (status) { case STATUS_RESOLVING: { ERR_FAIL_COND_V(resolving == IP::RESOLVER_INVALID_ID, ERR_BUG); diff --git a/core/io/stream_peer_tcp.cpp b/core/io/stream_peer_tcp.cpp index 6d5784a8ab4..6c3313b73b4 100644 --- a/core/io/stream_peer_tcp.cpp +++ b/core/io/stream_peer_tcp.cpp @@ -32,8 +32,28 @@ #include "core/config/project_settings.h" -Error StreamPeerTCP::_poll_connection() { - ERR_FAIL_COND_V(status != STATUS_CONNECTING || !_sock.is_valid() || !_sock->is_open(), FAILED); +Error StreamPeerTCP::poll() { + if (status == STATUS_CONNECTED) { + Error err; + err = _sock->poll(NetSocket::POLL_TYPE_IN, 0); + if (err == OK) { + // FIN received + if (_sock->get_available_bytes() == 0) { + disconnect_from_host(); + return OK; + } + } + // Also poll write + err = _sock->poll(NetSocket::POLL_TYPE_IN_OUT, 0); + if (err != OK && err != ERR_BUSY) { + // Got an error + disconnect_from_host(); + status = STATUS_ERROR; + return err; + } + } else if (status != STATUS_CONNECTING) { + return OK; + } Error err = _sock->connect_to_host(peer_host, peer_port); @@ -121,22 +141,7 @@ Error StreamPeerTCP::connect_to_host(const IPAddress &p_host, int p_port) { Error StreamPeerTCP::write(const uint8_t *p_data, int p_bytes, int &r_sent, bool p_block) { ERR_FAIL_COND_V(!_sock.is_valid(), ERR_UNAVAILABLE); - if (status == STATUS_NONE || status == STATUS_ERROR) { - return FAILED; - } - if (status != STATUS_CONNECTED) { - if (_poll_connection() != OK) { - return FAILED; - } - - if (status != STATUS_CONNECTED) { - r_sent = 0; - return OK; - } - } - - if (!_sock->is_open()) { return FAILED; } @@ -183,17 +188,6 @@ Error StreamPeerTCP::read(uint8_t *p_buffer, int p_bytes, int &r_received, bool return FAILED; } - if (status == STATUS_CONNECTING) { - if (_poll_connection() != OK) { - return FAILED; - } - - if (status != STATUS_CONNECTED) { - r_received = 0; - return OK; - } - } - Error err; int to_read = p_bytes; int total_read = 0; @@ -251,28 +245,7 @@ bool StreamPeerTCP::is_connected_to_host() const { return _sock.is_valid() && _sock->is_open() && (status == STATUS_CONNECTED || status == STATUS_CONNECTING); } -StreamPeerTCP::Status StreamPeerTCP::get_status() { - if (status == STATUS_CONNECTING) { - _poll_connection(); - } else if (status == STATUS_CONNECTED) { - Error err; - err = _sock->poll(NetSocket::POLL_TYPE_IN, 0); - if (err == OK) { - // FIN received - if (_sock->get_available_bytes() == 0) { - disconnect_from_host(); - return status; - } - } - // Also poll write - err = _sock->poll(NetSocket::POLL_TYPE_IN_OUT, 0); - if (err != OK && err != ERR_BUSY) { - // Got an error - disconnect_from_host(); - status = STATUS_ERROR; - } - } - +StreamPeerTCP::Status StreamPeerTCP::get_status() const { return status; } @@ -287,7 +260,7 @@ void StreamPeerTCP::disconnect_from_host() { peer_port = 0; } -Error StreamPeerTCP::poll(NetSocket::PollType p_type, int timeout) { +Error StreamPeerTCP::wait(NetSocket::PollType p_type, int timeout) { ERR_FAIL_COND_V(_sock.is_null() || !_sock->is_open(), ERR_UNAVAILABLE); return _sock->poll(p_type, timeout); } @@ -347,6 +320,7 @@ void StreamPeerTCP::_bind_methods() { ClassDB::bind_method(D_METHOD("bind", "port", "host"), &StreamPeerTCP::bind, DEFVAL("*")); ClassDB::bind_method(D_METHOD("connect_to_host", "host", "port"), &StreamPeerTCP::_connect); ClassDB::bind_method(D_METHOD("is_connected_to_host"), &StreamPeerTCP::is_connected_to_host); + ClassDB::bind_method(D_METHOD("poll"), &StreamPeerTCP::poll); ClassDB::bind_method(D_METHOD("get_status"), &StreamPeerTCP::get_status); ClassDB::bind_method(D_METHOD("get_connected_host"), &StreamPeerTCP::get_connected_host); ClassDB::bind_method(D_METHOD("get_connected_port"), &StreamPeerTCP::get_connected_port); diff --git a/core/io/stream_peer_tcp.h b/core/io/stream_peer_tcp.h index f2c47b25cf4..5fe1734faa6 100644 --- a/core/io/stream_peer_tcp.h +++ b/core/io/stream_peer_tcp.h @@ -56,7 +56,6 @@ protected: uint16_t peer_port = 0; Error _connect(const String &p_address, int p_port); - Error _poll_connection(); Error write(const uint8_t *p_data, int p_bytes, int &r_sent, bool p_block); Error read(uint8_t *p_buffer, int p_bytes, int &r_received, bool p_block); @@ -74,12 +73,15 @@ public: void disconnect_from_host(); int get_available_bytes() const override; - Status get_status(); + Status get_status() const; void set_no_delay(bool p_enabled); - // Poll functions (wait or check for writable, readable) - Error poll(NetSocket::PollType p_type, int timeout = 0); + // Poll socket updating its state. + Error poll(); + + // Wait or check for writable, readable. + Error wait(NetSocket::PollType p_type, int timeout = 0); // Read/Write from StreamPeer Error put_data(const uint8_t *p_data, int p_bytes) override; diff --git a/doc/classes/StreamPeerTCP.xml b/doc/classes/StreamPeerTCP.xml index a1f1f1be799..033982ea400 100644 --- a/doc/classes/StreamPeerTCP.xml +++ b/doc/classes/StreamPeerTCP.xml @@ -51,7 +51,7 @@ Returns the local port to which this peer is bound. - + Returns the status of the connection, see [enum Status]. @@ -63,6 +63,12 @@ Returns [code]true[/code] if this peer is currently connected or is connecting to a host, [code]false[/code] otherwise. + + + + Poll the socket, updating its state. See [method get_status]. + + diff --git a/modules/websocket/wsl_client.cpp b/modules/websocket/wsl_client.cpp index 1ef571b6ee7..96763037e76 100644 --- a/modules/websocket/wsl_client.cpp +++ b/modules/websocket/wsl_client.cpp @@ -273,6 +273,7 @@ void WSLClient::poll() { return; // Not connected. } + _tcp->poll(); switch (_tcp->get_status()) { case StreamPeerTCP::STATUS_NONE: // Clean close From 331f1662df41b4e7afff30d2fe4019a413499c1c Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Sun, 27 Mar 2022 16:31:56 +0200 Subject: [PATCH 2/2] [Net] Drop is_connected_to_host for TCP and UDP. The UDP method is now called `is_socket_connected` to limit confusion with the actual host connection status which doesn't make sense in UDP. The TCP method is completly dropped, use get_status instead. The only one left is the WebSocketPeer one, which should be fine as is for now. --- core/io/packet_peer_udp.cpp | 4 ++-- core/io/packet_peer_udp.h | 2 +- core/io/stream_peer_tcp.cpp | 9 ++------- core/io/stream_peer_tcp.h | 1 - doc/classes/PacketPeerUDP.xml | 2 +- doc/classes/StreamPeerTCP.xml | 6 ------ modules/mbedtls/packet_peer_mbed_dtls.cpp | 2 +- modules/websocket/wsl_client.cpp | 2 +- 8 files changed, 8 insertions(+), 20 deletions(-) diff --git a/core/io/packet_peer_udp.cpp b/core/io/packet_peer_udp.cpp index 503eb17cae2..84d1f3ebd4e 100644 --- a/core/io/packet_peer_udp.cpp +++ b/core/io/packet_peer_udp.cpp @@ -242,7 +242,7 @@ Error PacketPeerUDP::connect_to_host(const IPAddress &p_host, int p_port) { return OK; } -bool PacketPeerUDP::is_connected_to_host() const { +bool PacketPeerUDP::is_socket_connected() const { return connected; } @@ -348,7 +348,7 @@ void PacketPeerUDP::_bind_methods() { ClassDB::bind_method(D_METHOD("wait"), &PacketPeerUDP::wait); ClassDB::bind_method(D_METHOD("is_bound"), &PacketPeerUDP::is_bound); ClassDB::bind_method(D_METHOD("connect_to_host", "host", "port"), &PacketPeerUDP::connect_to_host); - ClassDB::bind_method(D_METHOD("is_connected_to_host"), &PacketPeerUDP::is_connected_to_host); + ClassDB::bind_method(D_METHOD("is_socket_connected"), &PacketPeerUDP::is_socket_connected); ClassDB::bind_method(D_METHOD("get_packet_ip"), &PacketPeerUDP::_get_packet_ip); ClassDB::bind_method(D_METHOD("get_packet_port"), &PacketPeerUDP::get_packet_port); ClassDB::bind_method(D_METHOD("get_local_port"), &PacketPeerUDP::get_local_port); diff --git a/core/io/packet_peer_udp.h b/core/io/packet_peer_udp.h index 444a6dd5ba1..a174cf53479 100644 --- a/core/io/packet_peer_udp.h +++ b/core/io/packet_peer_udp.h @@ -79,7 +79,7 @@ public: void disconnect_shared_socket(); // Used by UDPServer Error store_packet(IPAddress p_ip, uint32_t p_port, uint8_t *p_buf, int p_buf_size); // Used internally and by UDPServer Error connect_to_host(const IPAddress &p_host, int p_port); - bool is_connected_to_host() const; + bool is_socket_connected() const; IPAddress get_packet_address() const; int get_packet_port() const; diff --git a/core/io/stream_peer_tcp.cpp b/core/io/stream_peer_tcp.cpp index 6c3313b73b4..c5c2021e6e9 100644 --- a/core/io/stream_peer_tcp.cpp +++ b/core/io/stream_peer_tcp.cpp @@ -184,7 +184,7 @@ Error StreamPeerTCP::write(const uint8_t *p_data, int p_bytes, int &r_sent, bool } Error StreamPeerTCP::read(uint8_t *p_buffer, int p_bytes, int &r_received, bool p_block) { - if (!is_connected_to_host()) { + if (status != STATUS_CONNECTED) { return FAILED; } @@ -237,14 +237,10 @@ Error StreamPeerTCP::read(uint8_t *p_buffer, int p_bytes, int &r_received, bool } void StreamPeerTCP::set_no_delay(bool p_enabled) { - ERR_FAIL_COND(!is_connected_to_host()); + ERR_FAIL_COND(!_sock.is_valid() || !_sock->is_open()); _sock->set_tcp_no_delay_enabled(p_enabled); } -bool StreamPeerTCP::is_connected_to_host() const { - return _sock.is_valid() && _sock->is_open() && (status == STATUS_CONNECTED || status == STATUS_CONNECTING); -} - StreamPeerTCP::Status StreamPeerTCP::get_status() const { return status; } @@ -319,7 +315,6 @@ Error StreamPeerTCP::_connect(const String &p_address, int p_port) { void StreamPeerTCP::_bind_methods() { ClassDB::bind_method(D_METHOD("bind", "port", "host"), &StreamPeerTCP::bind, DEFVAL("*")); ClassDB::bind_method(D_METHOD("connect_to_host", "host", "port"), &StreamPeerTCP::_connect); - ClassDB::bind_method(D_METHOD("is_connected_to_host"), &StreamPeerTCP::is_connected_to_host); ClassDB::bind_method(D_METHOD("poll"), &StreamPeerTCP::poll); ClassDB::bind_method(D_METHOD("get_status"), &StreamPeerTCP::get_status); ClassDB::bind_method(D_METHOD("get_connected_host"), &StreamPeerTCP::get_connected_host); diff --git a/core/io/stream_peer_tcp.h b/core/io/stream_peer_tcp.h index 5fe1734faa6..f9eefb06040 100644 --- a/core/io/stream_peer_tcp.h +++ b/core/io/stream_peer_tcp.h @@ -66,7 +66,6 @@ public: Error bind(int p_port, const IPAddress &p_host); Error connect_to_host(const IPAddress &p_host, int p_port); - bool is_connected_to_host() const; IPAddress get_connected_host() const; int get_connected_port() const; int get_local_port() const; diff --git a/doc/classes/PacketPeerUDP.xml b/doc/classes/PacketPeerUDP.xml index 580bf605185..7c6622be3cf 100644 --- a/doc/classes/PacketPeerUDP.xml +++ b/doc/classes/PacketPeerUDP.xml @@ -61,7 +61,7 @@ Returns whether this [PacketPeerUDP] is bound to an address and can receive packets. - + Returns [code]true[/code] if the UDP socket is open and has been connected to a remote address. See [method connect_to_host]. diff --git a/doc/classes/StreamPeerTCP.xml b/doc/classes/StreamPeerTCP.xml index 033982ea400..f06cf5c7a24 100644 --- a/doc/classes/StreamPeerTCP.xml +++ b/doc/classes/StreamPeerTCP.xml @@ -57,12 +57,6 @@ Returns the status of the connection, see [enum Status]. - - - - Returns [code]true[/code] if this peer is currently connected or is connecting to a host, [code]false[/code] otherwise. - - diff --git a/modules/mbedtls/packet_peer_mbed_dtls.cpp b/modules/mbedtls/packet_peer_mbed_dtls.cpp index 2a5eaa0109a..9676f16b2d8 100644 --- a/modules/mbedtls/packet_peer_mbed_dtls.cpp +++ b/modules/mbedtls/packet_peer_mbed_dtls.cpp @@ -115,7 +115,7 @@ Error PacketPeerMbedDTLS::_do_handshake() { } Error PacketPeerMbedDTLS::connect_to_peer(Ref p_base, bool p_validate_certs, const String &p_for_hostname, Ref p_ca_certs) { - ERR_FAIL_COND_V(!p_base.is_valid() || !p_base->is_connected_to_host(), ERR_INVALID_PARAMETER); + ERR_FAIL_COND_V(!p_base.is_valid() || !p_base->is_socket_connected(), ERR_INVALID_PARAMETER); base = p_base; int ret = 0; diff --git a/modules/websocket/wsl_client.cpp b/modules/websocket/wsl_client.cpp index 96763037e76..58c329f0432 100644 --- a/modules/websocket/wsl_client.cpp +++ b/modules/websocket/wsl_client.cpp @@ -337,7 +337,7 @@ MultiplayerPeer::ConnectionStatus WSLClient::get_connection_status() const { return CONNECTION_CONNECTED; } - if (_tcp->is_connected_to_host() || _resolver_id != IP::RESOLVER_INVALID_ID) { + if (_tcp->get_status() == StreamPeerTCP::STATUS_CONNECTING || _resolver_id != IP::RESOLVER_INVALID_ID) { return CONNECTION_CONNECTING; }