Skip to content

Commit

Permalink
HG util: prevent locking in hg_request_wait()
Browse files Browse the repository at this point in the history
Concurrent progress in multi-threaded scenarios on the same context
could complete another thread's request and let it sit in progress
  • Loading branch information
soumagne committed Oct 19, 2023
1 parent 5f14645 commit 935264b
Showing 1 changed file with 8 additions and 34 deletions.
42 changes: 8 additions & 34 deletions src/util/mercury_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
*/

#include "mercury_request.h"
#include "mercury_thread_condition.h"
#include "mercury_thread_mutex.h"
#include "mercury_param.h"
#include "mercury_time.h"
#include "mercury_util_error.h"

Expand All @@ -17,6 +16,8 @@
/* Local Macros */
/****************/

#define HG_REQUEST_PROGRESS_TIMEOUT (500)

/************************************/
/* Local Type and Struct Definition */
/************************************/
Expand All @@ -25,9 +26,6 @@ struct hg_request_class {
hg_request_progress_func_t progress_func;
hg_request_trigger_func_t trigger_func;
void *arg;
bool progressing;
hg_thread_mutex_t progress_mutex;
hg_thread_cond_t progress_cond;
};

/********************/
Expand All @@ -53,9 +51,6 @@ hg_request_init(hg_request_progress_func_t progress_func,
hg_request_class->progress_func = progress_func;
hg_request_class->trigger_func = trigger_func;
hg_request_class->arg = arg;
hg_request_class->progressing = false;
hg_thread_mutex_init(&hg_request_class->progress_mutex);
hg_thread_cond_init(&hg_request_class->progress_cond);

done:
return hg_request_class;
Expand All @@ -70,8 +65,7 @@ hg_request_finalize(hg_request_class_t *request_class, void **arg)

if (arg)
*arg = request_class->arg;
hg_thread_mutex_destroy(&request_class->progress_mutex);
hg_thread_cond_destroy(&request_class->progress_cond);

free(request_class);
}

Expand Down Expand Up @@ -115,7 +109,8 @@ hg_request_wait(
deadline = hg_time_add(now, remaining);

do {
unsigned int trigger_flag = 0;
unsigned int trigger_flag = 0,
progress_timeout = hg_time_to_ms(remaining);
int trigger_ret;

do {
Expand All @@ -127,31 +122,10 @@ hg_request_wait(
if (completed)
break;

hg_thread_mutex_lock(&request->request_class->progress_mutex);
if (request->request_class->progressing) {
if (hg_thread_cond_timedwait(&request->request_class->progress_cond,
&request->request_class->progress_mutex,
hg_time_to_ms(remaining)) != HG_UTIL_SUCCESS) {
/* Timeout occurred so leave */
hg_thread_mutex_unlock(&request->request_class->progress_mutex);
break;
}
/* Continue as request may have completed in the meantime */
hg_thread_mutex_unlock(&request->request_class->progress_mutex);
goto next;
}
request->request_class->progressing = true;
hg_thread_mutex_unlock(&request->request_class->progress_mutex);

request->request_class->progress_func(
hg_time_to_ms(remaining), request->request_class->arg);

hg_thread_mutex_lock(&request->request_class->progress_mutex);
request->request_class->progressing = false;
hg_thread_cond_broadcast(&request->request_class->progress_cond);
hg_thread_mutex_unlock(&request->request_class->progress_mutex);
MIN(progress_timeout, HG_REQUEST_PROGRESS_TIMEOUT),
request->request_class->arg);

next:
if (timeout_ms != 0)
hg_time_get_current_ms(&now);
remaining = hg_time_subtract(deadline, now);
Expand Down

0 comments on commit 935264b

Please sign in to comment.