Skip to content

Commit

Permalink
fix: partially fix a bug that prevented group part messages from
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
JFreegman committed Jan 27, 2024
1 parent eeaa039 commit b0aea1c
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 10 deletions.
9 changes: 6 additions & 3 deletions auto_tests/group_general_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
2 changes: 1 addition & 1 deletion other/bootstrap_daemon/docker/tox-bootstrapd.sha256
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7dfcf534fb80fbd8337337f5aa9eaa120febc72386046c7ab0d5c7545e900657 /usr/local/bin/tox-bootstrapd
e5ccf844b7ece48d1153c226bdf8462d0e85d517dfcd720c8d6c4abad7da6929 /usr/local/bin/tox-bootstrapd
21 changes: 15 additions & 6 deletions toxcore/group_chats.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -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");

Check warning on line 8277 in toxcore/group_chats.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_chats.c#L8277

Added line #L8277 was not covered by tests
}
}
}

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(1, 2) nullable(3)
static int kill_group(GC_Session *c, GC_Chat *chat, const uint8_t *message, uint16_t length)
{
const int ret = gc_group_exit(c, chat, message, length);
group_delete(c, chat);
return ret;
}
Expand All @@ -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, nullptr, 0) != 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);
Expand Down
2 changes: 2 additions & 0 deletions toxcore/group_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b0aea1c

Please sign in to comment.