From 4e0b2e21813e4c0978dcfc10b9df2c36b97868bf Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Tue, 7 May 2024 10:49:56 -0700 Subject: [PATCH] Fix signed/unsigned bug with aws_future_wait() timeout value (#638) --- source/future.c | 5 ++++- tests/CMakeLists.txt | 1 + tests/future_test.c | 23 +++++++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/source/future.c b/source/future.c index be213184b..96c88ef6a 100644 --- a/source/future.c +++ b/source/future.c @@ -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; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9856a998b..21acdfdc5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -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) diff --git a/tests/future_test.c b/tests/future_test.c index 45dfa7cea..1ac94b551 100644 --- a/tests/future_test.c +++ b/tests/future_test.c @@ -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;