From e5129c574db2fa98834f8fdc82c6a6f056363155 Mon Sep 17 00:00:00 2001 From: Dongha Kim Date: Fri, 20 Dec 2024 16:17:50 -0700 Subject: [PATCH] Use shutdown_socket() in rti_remote.c and federate.c --- core/federated/RTI/rti_remote.c | 51 ++++---------------------- core/federated/federate.c | 21 ++--------- core/federated/network/socket_common.c | 28 ++++++++------ 3 files changed, 28 insertions(+), 72 deletions(-) diff --git a/core/federated/RTI/rti_remote.c b/core/federated/RTI/rti_remote.c index c5bd02955..34075d917 100644 --- a/core/federated/RTI/rti_remote.c +++ b/core/federated/RTI/rti_remote.c @@ -871,14 +871,7 @@ static void handle_federate_failed(federate_info_t* my_fed) { // Indicate that there will no further events from this federate. my_fed->enclave.next_event = FOREVER_TAG; - // According to this: https://stackoverflow.com/questions/4160347/close-vs-shutdown-socket, - // the close should happen when receiving a 0 length message from the other end. - // Here, we just signal the other side that no further writes to the socket are - // forthcoming, which should result in the other end getting a zero-length reception. - shutdown(my_fed->socket, SHUT_RDWR); - - // We can now safely close the socket. - close(my_fed->socket); // from unistd.h + shutdown_socket(&my_fed->socket, false); // Check downstream federates to see whether they should now be granted a TAG. // To handle cycles, need to create a boolean array to keep @@ -917,21 +910,7 @@ static void handle_federate_resign(federate_info_t* my_fed) { // Indicate that there will no further events from this federate. my_fed->enclave.next_event = FOREVER_TAG; - // According to this: https://stackoverflow.com/questions/4160347/close-vs-shutdown-socket, - // the close should happen when receiving a 0 length message from the other end. - // Here, we just signal the other side that no further writes to the socket are - // forthcoming, which should result in the other end getting a zero-length reception. - shutdown(my_fed->socket, SHUT_WR); - - // Wait for the federate to send an EOF or a socket error to occur. - // Discard any incoming bytes. Normally, this read should return 0 because - // the federate is resigning and should itself invoke shutdown. - unsigned char buffer[10]; - while (read(my_fed->socket, buffer, 10) > 0) - ; - - // We can now safely close the socket. - close(my_fed->socket); // from unistd.h + shutdown_socket(&my_fed->socket, true); // Check downstream federates to see whether they should now be granted a TAG. // To handle cycles, need to create a boolean array to keep @@ -1030,9 +1009,7 @@ void send_reject(int* socket_id, unsigned char error_code) { lf_print_warning("RTI failed to write MSG_TYPE_REJECT message on the socket."); } // Close the socket. - shutdown(*socket_id, SHUT_RDWR); - close(*socket_id); - *socket_id = -1; + shutdown_socket(socket_id, false); LF_MUTEX_UNLOCK(&rti_mutex); } @@ -1420,9 +1397,7 @@ void lf_connect_to_federates(int socket_descriptor) { if (!authenticate_federate(&socket_id)) { lf_print_warning("RTI failed to authenticate the incoming federate."); // Close the socket. - shutdown(socket_id, SHUT_RDWR); - close(socket_id); - socket_id = -1; + shutdown_socket(&socket_id, false); // Ignore the federate that failed authentication. i--; continue; @@ -1490,8 +1465,7 @@ void* respond_to_erroneous_connections(void* nothing) { lf_print_warning("RTI failed to write FEDERATION_ID_DOES_NOT_MATCH to erroneous incoming connection."); } // Close the socket. - shutdown(socket_id, SHUT_RDWR); - close(socket_id); + shutdown_socket(&socket_id, false); } return NULL; } @@ -1554,21 +1528,10 @@ void wait_for_federates(int socket_descriptor) { // Shutdown and close the socket that is listening for incoming connections // so that the accept() call in respond_to_erroneous_connections returns. // That thread should then check rti->all_federates_exited and it should exit. - if (shutdown(socket_descriptor, SHUT_RDWR)) { - LF_PRINT_LOG("On shut down TCP socket, received reply: %s", strerror(errno)); - } - // NOTE: In all common TCP/IP stacks, there is a time period, - // typically between 30 and 120 seconds, called the TIME_WAIT period, - // before the port is released after this close. This is because - // the OS is preventing another program from accidentally receiving - // duplicated packets intended for this program. - close(socket_descriptor); + shutdown_socket(&socket_descriptor, false); if (rti_remote->socket_descriptor_UDP > 0) { - if (shutdown(rti_remote->socket_descriptor_UDP, SHUT_RDWR)) { - LF_PRINT_LOG("On shut down UDP socket, received reply: %s", strerror(errno)); - } - close(rti_remote->socket_descriptor_UDP); + shutdown_socket(&rti_remote->socket_descriptor_UDP, false); } } diff --git a/core/federated/federate.c b/core/federated/federate.c index 4b12f8b53..dbdb691c1 100644 --- a/core/federated/federate.c +++ b/core/federated/federate.c @@ -417,14 +417,12 @@ static void close_inbound_socket(int fed_id, int flag) { if (_fed.sockets_for_inbound_p2p_connections[fed_id] >= 0) { if (flag >= 0) { if (flag > 0) { - shutdown(_fed.sockets_for_inbound_p2p_connections[fed_id], SHUT_RDWR); + shutdown_socket(&_fed.sockets_for_inbound_p2p_connections[fed_id], false); } else { // Have received EOF from the other end. Send EOF to the other end. - shutdown(_fed.sockets_for_inbound_p2p_connections[fed_id], SHUT_WR); + shutdown_socket(&_fed.sockets_for_inbound_p2p_connections[fed_id], true); } } - close(_fed.sockets_for_inbound_p2p_connections[fed_id]); - _fed.sockets_for_inbound_p2p_connections[fed_id] = -1; } LF_MUTEX_UNLOCK(&socket_mutex); } @@ -837,20 +835,9 @@ static void close_outbound_socket(int fed_id, int flag) { if (_fed.sockets_for_outbound_p2p_connections[fed_id] >= 0) { // Close the socket by sending a FIN packet indicating that no further writes // are expected. Then read until we get an EOF indication. - if (flag >= 0) { - // SHUT_WR indicates no further outgoing messages. - shutdown(_fed.sockets_for_outbound_p2p_connections[fed_id], SHUT_WR); - if (flag > 0) { - // Have not received EOF yet. read until we get an EOF or error indication. - // This compensates for delayed ACKs and disabling of Nagles algorithm - // by delaying exiting until the shutdown is complete. - unsigned char message[32]; - while (read(_fed.sockets_for_outbound_p2p_connections[fed_id], &message, 32) > 0) - ; - } + if (flag > 0) { + shutdown_socket(&_fed.sockets_for_outbound_p2p_connections[fed_id], true); } - close(_fed.sockets_for_outbound_p2p_connections[fed_id]); - _fed.sockets_for_outbound_p2p_connections[fed_id] = -1; } if (_lf_normal_termination) { LF_MUTEX_UNLOCK(&lf_outbound_socket_mutex); diff --git a/core/federated/network/socket_common.c b/core/federated/network/socket_common.c index 4fd5f319b..6d12a8657 100644 --- a/core/federated/network/socket_common.c +++ b/core/federated/network/socket_common.c @@ -313,10 +313,7 @@ int read_from_socket_close_on_error(int* socket, size_t num_bytes, unsigned char // Read failed. // Socket has probably been closed from the other side. // Shut down and close the socket from this side. - shutdown(*socket, SHUT_RDWR); - close(*socket); - // Mark the socket closed. - *socket = -1; + shutdown_socket(socket, false); return -1; } return 0; @@ -383,10 +380,7 @@ int write_to_socket_close_on_error(int* socket, size_t num_bytes, unsigned char* // Write failed. // Socket has probably been closed from the other side. // Shut down and close the socket from this side. - shutdown(*socket, SHUT_RDWR); - close(*socket); - // Mark the socket closed. - *socket = -1; + shutdown_socket(socket, false); } return result; } @@ -414,7 +408,7 @@ void write_to_socket_fail_on_error(int* socket, size_t num_bytes, unsigned char* int shutdown_socket(int* socket, bool read_before_closing) { if (!read_before_closing) { if (shutdown(*socket, SHUT_RDWR)) { - lf_print_log("On shut down TCP socket, received reply: %s", strerror(errno)); + lf_print_warning("On shut down TCP socket, received reply: %s", strerror(errno)); return -1; } } else { @@ -422,20 +416,32 @@ int shutdown_socket(int* socket, bool read_before_closing) { // the close should happen when receiving a 0 length message from the other end. // Here, we just signal the other side that no further writes to the socket are // forthcoming, which should result in the other end getting a zero-length reception. + + // Close the socket by sending a FIN packet indicating that no further writes + // are expected. Then read until we get an EOF indication. if (shutdown(*socket, SHUT_WR)) { - lf_print_log("On shut down TCP socket, received reply: %s", strerror(errno)); + lf_print_warning("On shut down socket, received reply: %s", strerror(errno)); return -1; } // Wait for the other end to send an EOF or a socket error to occur. // Discard any incoming bytes. Normally, this read should return 0 because // the federate is resigning and should itself invoke shutdown. + + // Have not received EOF yet. read until we get an EOF or error indication. + // This compensates for delayed ACKs and disabling of Nagles algorithm + // by delaying exiting until the shutdown is complete. unsigned char buffer[10]; while (read(*socket, buffer, 10) > 0) ; } + // NOTE: In all common TCP/IP stacks, there is a time period, + // typically between 30 and 120 seconds, called the TIME_WAIT period, + // before the port is released after this close. This is because + // the OS is preventing another program from accidentally receiving + // duplicated packets intended for this program. if (close(*socket)) { - lf_print_log("Error while closing socket: %s\n", strerror(errno)); + lf_print_warning("Error while closing socket: %s\n", strerror(errno)); return -1; } *socket = -1;