Skip to content

Commit 8063ceb

Browse files
committed
ultragrid_rtp: Fix data race on exit
send_video_frame_async_callback() could try to use s->async_sending_cv after it was destroyed in done(). This is because while join() waits for the sending to be finished, the cond_signal happens after the mutex unlock and therefore is not protected. Also detached tasks are supposed to own the data they use, but this one didn't.
1 parent fc53e96 commit 8063ceb

1 file changed

Lines changed: 9 additions & 22 deletions

File tree

src/rxtx/ultragrid_rtp.c

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,7 @@ struct ultragrid_rtp_rxtx {
116116
/**
117117
* This variables serve as a notification when asynchronous sending exits
118118
* @{ */
119-
bool async_sending;
120-
pthread_cond_t async_sending_cv;
121-
pthread_mutex_t async_sending_lock;
119+
task_result_handle_t async_sending_task;
122120
/// @}
123121

124122
long long int send_bytes_total;
@@ -144,8 +142,6 @@ static void done(void *state)
144142
display_done(s->display_copies[i]);
145143
}
146144
rtp_rxtx_common_done(s->rtp_common);
147-
CHK_PTHR(pthread_cond_destroy(&s->async_sending_cv));
148-
CHK_PTHR(pthread_mutex_destroy(&s->async_sending_lock));
149145
free(s);
150146
}
151147

@@ -166,8 +162,7 @@ init(struct rxtx_params *params)
166162
s->parent = params->parent;
167163
s->start_time = params->start_time;
168164
s->receiver_mod = params->receiver_mod;
169-
ug_pthread_mutex_init(&s->async_sending_lock);
170-
pthread_cond_init(&s->async_sending_cv, nullptr);
165+
s->async_sending_task = nullptr;
171166
int rc = rtp_rxtx_common_init(&s->rtp_common, params);
172167
if (rc != 0) {
173168
done(s);
@@ -190,11 +185,10 @@ init(struct rxtx_params *params)
190185

191186
static void join(void *state) {
192187
struct ultragrid_rtp_rxtx *s = state;
193-
CHK_PTHR(pthread_mutex_lock(&s->async_sending_lock));
194-
while (s->async_sending) {
195-
pthread_cond_wait(&s->async_sending_cv, &s->async_sending_lock);
188+
if(s->async_sending_task){
189+
wait_task(s->async_sending_task);
190+
s->async_sending_task = nullptr;
196191
}
197-
CHK_PTHR(pthread_mutex_unlock(&s->async_sending_lock));
198192
}
199193

200194
struct async_data {
@@ -222,14 +216,12 @@ send_video_frame(void *state, struct video_frame *tx_frame)
222216
data->s = s;
223217
data->f = tx_frame;
224218

225-
CHK_PTHR(pthread_mutex_lock(&s->async_sending_lock));
226-
while (s->async_sending) {
227-
pthread_cond_wait(&s->async_sending_cv, &s->async_sending_lock);
219+
if(s->async_sending_task){
220+
wait_task(s->async_sending_task);
221+
s->async_sending_task = nullptr;
228222
}
229223
rtp_rxtx_sender_do_housekeeping(s->rtp_common, TX_MEDIA_VIDEO);
230-
s->async_sending = true;
231-
task_run_async_detached(send_video_frame_async_callback, (void *) data);
232-
CHK_PTHR(pthread_mutex_unlock(&s->async_sending_lock));
224+
s->async_sending_task = task_run_async(send_video_frame_async_callback, (void *) data);
233225
}
234226

235227
static void *send_video_frame_async_callback(void *arg) {
@@ -245,11 +237,6 @@ static void *send_video_frame_async_callback(void *arg) {
245237

246238
tx_frame->callbacks.dispose(tx_frame);
247239

248-
CHK_PTHR(pthread_mutex_lock(&s->async_sending_lock));
249-
s->async_sending = false;
250-
CHK_PTHR(pthread_mutex_unlock(&s->async_sending_lock));
251-
CHK_PTHR(pthread_cond_signal(&s->async_sending_cv));
252-
253240
return nullptr;
254241
}
255242

0 commit comments

Comments
 (0)