Skip to content

Commit

Permalink
Fix signed/unsigned bug with aws_future_wait() timeout value (#638)
Browse files Browse the repository at this point in the history
  • Loading branch information
graebm authored May 7, 2024
1 parent 7e04b86 commit 4e0b2e2
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 1 deletion.
5 changes: 4 additions & 1 deletion source/future.c
Original file line number Diff line number Diff line change
Expand Up @@ -453,13 +453,16 @@ bool aws_future_impl_wait(const struct aws_future_impl *future, uint64_t timeout
/* this function is conceptually const, but we need to use synchronization primitives */
struct aws_future_impl *mutable_future = (struct aws_future_impl *)future;

/* condition-variable takes signed timeout, so clamp to INT64_MAX (292+ years) */
int64_t timeout_i64 = aws_min_u64(timeout_ns, INT64_MAX);

/* BEGIN CRITICAL SECTION */
aws_mutex_lock(&mutable_future->lock);

bool is_done = aws_condition_variable_wait_for_pred(
&mutable_future->wait_cvar,
&mutable_future->lock,
(int64_t)timeout_ns,
timeout_i64,
s_future_impl_is_done_pred,
mutable_future) == AWS_OP_SUCCESS;

Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ add_test_case(future_register_event_loop_callback_before_done)
add_test_case(future_register_event_loop_callback_always_scheduled)
add_test_case(future_register_channel_callback)
add_test_case(future_wait_timeout)
add_test_case(future_wait_timeout_max)
add_test_case(future_pointer_with_destroy)
add_test_case(future_pointer_with_release)
add_test_case(future_get_result_by_move)
Expand Down
23 changes: 23 additions & 0 deletions tests/future_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,29 @@ static int s_test_future_wait_timeout(struct aws_allocator *alloc, void *ctx) {
}
AWS_TEST_CASE(future_wait_timeout, s_test_future_wait_timeout)

/* This is a regression test */
static int s_test_future_wait_timeout_max(struct aws_allocator *alloc, void *ctx) {
(void)ctx;
aws_io_library_init(alloc);

/* Thread will complete the future in 1sec */
struct aws_future_size *future = s_start_thread_job(alloc, ONE_SEC_IN_NS);

/* Wait for future to complete, with timeout of UINT64_MAX.
* Once upon a time, there was a bug where this became a negative number and immediately timed out. */
bool completed_before_timeout = aws_future_size_wait(future, UINT64_MAX);
ASSERT_TRUE(completed_before_timeout);

/* Wait until other thread joins, at which point the future is complete and the callback has fired */
aws_thread_set_managed_join_timeout_ns(MAX_TIMEOUT_NS);
ASSERT_SUCCESS(aws_thread_join_all_managed());

aws_future_size_release(future);
aws_io_library_clean_up();
return 0;
}
AWS_TEST_CASE(future_wait_timeout_max, s_test_future_wait_timeout_max)

struct aws_destroyme {
struct aws_allocator *alloc;
bool *set_true_on_death;
Expand Down

0 comments on commit 4e0b2e2

Please sign in to comment.