From d94246a9061e2af7c02c4edc94156f5e8148bd9a Mon Sep 17 00:00:00 2001 From: jfreegman Date: Sat, 27 Jan 2024 12:27:27 -0500 Subject: [PATCH] fix: partially fix a bug that prevented group part messages from sending. When a peer leaves a group, they send a packet to the group indicating that they're leaving. However if this packet is sent via TCP, it gets put in a packet queue, which is then destroyed on the rest of the group cleanup process before ever being able to send. This pr allows do_gc() to finish an iteration before cleaning the group up, which allows the TCP packet queue to be emptied. However this bug still exists on a tox_kill() event because we don't have a chance to do another do_gc() iteration. --- auto_tests/group_general_test.c | 9 +++++--- .../docker/tox-bootstrapd.sha256 | 2 +- toxcore/group_chats.c | 21 +++++++++++++------ toxcore/group_common.h | 2 ++ 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/auto_tests/group_general_test.c b/auto_tests/group_general_test.c index 7b2e215c..912261e9 100644 --- a/auto_tests/group_general_test.c +++ b/auto_tests/group_general_test.c @@ -441,10 +441,13 @@ static void group_announce_test(AutoTox *autotoxes) tox_group_leave(tox1, groupnumber, nullptr, 0, &err_exit); ck_assert(err_exit == TOX_ERR_GROUP_LEAVE_OK); - num_groups1 = tox_group_get_number_groups(tox0); - num_groups2 = tox_group_get_number_groups(tox1); + while (num_groups1 != 0 || num_groups2 != 0) { + num_groups1 = tox_group_get_number_groups(tox0); + num_groups2 = tox_group_get_number_groups(tox1); + + iterate_all_wait(autotoxes, NUM_GROUP_TOXES, ITERATION_INTERVAL); + } - ck_assert(num_groups1 == num_groups2 && num_groups2 == 0); printf("All tests passed!\n"); } diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 0f2faf44..9a8224f4 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -7dfcf534fb80fbd8337337f5aa9eaa120febc72386046c7ab0d5c7545e900657 /usr/local/bin/tox-bootstrapd +e5ccf844b7ece48d1153c226bdf8462d0e85d517dfcd720c8d6c4abad7da6929 /usr/local/bin/tox-bootstrapd diff --git a/toxcore/group_chats.c b/toxcore/group_chats.c index 6dd942ba..6b595c06 100644 --- a/toxcore/group_chats.c +++ b/toxcore/group_chats.c @@ -7281,6 +7281,10 @@ void do_gc(GC_Session *c, void *userdata) do_new_connection_cooldown(chat); do_peer_delete(c, chat, userdata); + + if (chat->flag_exit) { // should always come last as it modifies the chats array + group_delete(c, chat); + } } } @@ -8270,14 +8274,21 @@ static void group_delete(GC_Session *c, GC_Chat *chat) c->chats_index = i; if (!realloc_groupchats(c, c->chats_index)) { - LOGGER_ERROR(chat->log, "Failed to reallocate groupchats array"); + LOGGER_ERROR(c->messenger->log, "Failed to reallocate groupchats array"); } } } int gc_group_exit(GC_Session *c, GC_Chat *chat, const uint8_t *message, uint16_t length) { - const int ret = group_can_handle_packets(chat) ? send_gc_self_exit(chat, message, length) : 0; + chat->flag_exit = true; + return group_can_handle_packets(chat) ? send_gc_self_exit(chat, message, length) : 0; +} + +non_null() +static int kill_group(GC_Session *c, GC_Chat *chat) +{ + const int ret = gc_group_exit(c, chat, nullptr, 0); group_delete(c, chat); return ret; } @@ -8295,11 +8306,9 @@ void kill_dht_groupchats(GC_Session *c) continue; } - if (group_can_handle_packets(chat)) { - send_gc_self_exit(chat, nullptr, 0); + if (kill_group(c, chat) != 0) { + LOGGER_WARNING(c->messenger->log, "Failed to send group exit packet"); } - - group_cleanup(c, chat); } networking_registerhandler(c->messenger->net, NET_PACKET_GC_LOSSY, nullptr, nullptr); diff --git a/toxcore/group_common.h b/toxcore/group_common.h index dbbd7441..1db66c04 100644 --- a/toxcore/group_common.h +++ b/toxcore/group_common.h @@ -321,6 +321,8 @@ typedef struct GC_Chat { uint8_t m_group_public_key[CRYPTO_PUBLIC_KEY_SIZE]; // public key for group's messenger friend connection int friend_connection_id; // identifier for group's messenger friend connection + + bool flag_exit; // true if the group will be deleted after the next do_gc() iteration } GC_Chat; #ifndef MESSENGER_DEFINED