Skip to content

Commit

Permalink
fix: race condition when drawing group/conference peer list
Browse files Browse the repository at this point in the history
The number of peers can change between and within iterations leading to
an out of bounds index. Now we just lock the entire loop, which is less
efficient but necessary.
  • Loading branch information
JFreegman committed Jun 24, 2024
1 parent 8791b47 commit b863eee
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 20 deletions.
20 changes: 9 additions & 11 deletions src/conference.c
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,6 @@ static bool conference_onKey(ToxWindow *self, Toxic *toxic, wint_t key, bool ltr

static void draw_peer(ToxWindow *self, Toxic *toxic, ChatContext *ctx, uint32_t i)
{
pthread_mutex_lock(&Winthread.lock);
const uint32_t peer_idx = i + conferences[self->num].side_pos;
const uint32_t peernum = conferences[self->num].name_list[peer_idx].peernum;
const bool is_self = tox_conference_peer_number_is_ours(toxic->tox, self->num, peernum, NULL);
Expand All @@ -1083,26 +1082,20 @@ static void draw_peer(ToxWindow *self, Toxic *toxic, ChatContext *ctx, uint32_t
(is_self
? device_is_muted(input, conferences[self->num].audio_in_idx)
: peer != NULL && device_is_muted(output, peer->audio_out_idx));
pthread_mutex_unlock(&Winthread.lock);

const int aud_attr = A_BOLD | COLOR_PAIR(audio_active && !mute ? GREEN : RED);
wattron(ctx->sidebar, aud_attr);
waddch(ctx->sidebar, audio_active ? (mute ? 'M' : '*') : '-');
wattroff(ctx->sidebar, aud_attr);
waddch(ctx->sidebar, ' ');
#endif
} else {
pthread_mutex_unlock(&Winthread.lock);
}

/* truncate nick to fit in side panel without modifying list */
char tmpnick[TOX_MAX_NAME_LENGTH];
const int maxlen = SIDEBAR_WIDTH - 2 - 2 * audio;

pthread_mutex_lock(&Winthread.lock);
memcpy(tmpnick, &conferences[self->num].name_list[peer_idx].name, maxlen);
pthread_mutex_unlock(&Winthread.lock);

tmpnick[maxlen] = '\0';

if (is_self) {
Expand Down Expand Up @@ -1159,7 +1152,6 @@ static void conference_onDraw(ToxWindow *self, Toxic *toxic)
wattroff(ctx->sidebar, COLOR_PAIR(PEERLIST_LINE));

pthread_mutex_lock(&Winthread.lock);
const uint32_t num_peers = chat->num_peers;
const bool audio = chat->audio_enabled;
const int header_lines = sidebar_offset(self->num);
pthread_mutex_unlock(&Winthread.lock);
Expand Down Expand Up @@ -1210,6 +1202,10 @@ static void conference_onDraw(ToxWindow *self, Toxic *toxic)
#endif // AUDIO
}

pthread_mutex_lock(&Winthread.lock);
const uint32_t num_peers = chat->num_peers;
pthread_mutex_unlock(&Winthread.lock);

wmove(ctx->sidebar, line, 1);
wattron(ctx->sidebar, A_BOLD);
wprintw(ctx->sidebar, "Peers: %"PRIu32"\n", num_peers);
Expand All @@ -1221,12 +1217,14 @@ static void conference_onDraw(ToxWindow *self, Toxic *toxic)
mvwhline(ctx->sidebar, line, 1, ACS_HLINE, SIDEBAR_WIDTH - 1);
wattroff(ctx->sidebar, COLOR_PAIR(PEERLIST_LINE));

for (uint32_t i = 0;
i < num_peers && i < y2 - header_lines - CHATBOX_HEIGHT;
++i) {
pthread_mutex_lock(&Winthread.lock);

for (uint32_t i = 0; i < chat->num_peers && i < y2 - header_lines - CHATBOX_HEIGHT; ++i) {
wmove(ctx->sidebar, i + header_lines, 1);
draw_peer(self, toxic, ctx, i);
}

pthread_mutex_unlock(&Winthread.lock);
}

int y, x;
Expand Down
12 changes: 3 additions & 9 deletions src/groupchats.c
Original file line number Diff line number Diff line change
Expand Up @@ -2145,15 +2145,9 @@ static void groupchat_onDraw(ToxWindow *self, Toxic *toxic)
uint32_t offset = 0;

pthread_mutex_lock(&Winthread.lock);
const uint32_t max_idx = chat->max_idx;
const uint32_t start = chat->side_pos;
pthread_mutex_unlock(&Winthread.lock);

for (uint32_t i = start; i < max_idx && offset < maxlines; ++i) {
pthread_mutex_lock(&Winthread.lock);

for (uint32_t i = chat->side_pos; i < chat->max_idx && offset < maxlines; ++i) {
if (!chat->peer_list[i].active) {
pthread_mutex_unlock(&Winthread.lock);
continue;
}

Expand Down Expand Up @@ -2194,8 +2188,6 @@ static void groupchat_onDraw(ToxWindow *self, Toxic *toxic)
rolecolour = MAGENTA;
}

pthread_mutex_unlock(&Winthread.lock);

if (is_ignored) {
wattron(ctx->sidebar, COLOR_PAIR(RED) | A_BOLD);
wprintw(ctx->sidebar, "#");
Expand All @@ -2214,6 +2206,8 @@ static void groupchat_onDraw(ToxWindow *self, Toxic *toxic)
}
}

pthread_mutex_unlock(&Winthread.lock);

int y;
int x;
getyx(self->window, y, x);
Expand Down

0 comments on commit b863eee

Please sign in to comment.