Remove non portable use of pthread_t (#563)

Closes #562

* Add platform-specific typedef `aws_thread_id_t` and portable printing/comparison functions
* Logging uses thread-local storage to avoid recomputing thread id string

include/aws/common/logging.h

@@ -17,6 +17,7 @@
  */
 
 #include <aws/common/common.h>
+#include <aws/common/thread.h>
 
 #define AWS_LOG_LEVEL_NONE 0
 #define AWS_LOG_LEVEL_FATAL 1
@@ -237,6 +238,16 @@ void aws_logger_clean_up(struct aws_logger *logger);
 AWS_COMMON_API
 int aws_log_level_to_string(enum aws_log_level log_level, const char **level_string);
 
+/**
+ * Converts an aws_thread_id_t to a c-string.  For portability, aws_thread_id_t
+ * must not be printed directly.  Intended primarily to support building log
+ * lines that include the thread id in them.  The parameter `buffer` must
+ * point-to a char buffer of length `bufsz == AWS_THREAD_ID_T_REPR_BUFSZ`.  The
+ * thread id representation is returned in `buffer`.
+ */
+AWS_COMMON_API
+int aws_thread_id_t_to_string(aws_thread_id_t thread_id, char *buffer, size_t bufsz);
+
 /**
  * Get subject name from log subject.
  */

include/aws/common/thread.h

@@ -37,20 +37,27 @@ typedef union {
 } aws_thread_once;
 #    define AWS_THREAD_ONCE_STATIC_INIT                                                                                \
         { NULL }
+typedef unsigned long aws_thread_id_t;
 #else
 typedef pthread_once_t aws_thread_once;
 #    define AWS_THREAD_ONCE_STATIC_INIT PTHREAD_ONCE_INIT
+typedef pthread_t aws_thread_id_t;
 #endif
 
+/*
+ * Buffer size needed to represent aws_thread_id_t as a string (2 hex chars per byte
+ * plus '\0' terminator). Needed for portable printing because pthread_t is
+ * opaque.
+ */
+#define AWS_THREAD_ID_T_REPR_BUFSZ (sizeof(aws_thread_id_t) * 2 + 1)
+
 struct aws_thread {
     struct aws_allocator *allocator;
     enum aws_thread_detach_state detach_state;
 #ifdef _WIN32
     void *thread_handle;
-    unsigned long thread_id;
-#else
-    pthread_t thread_id;
 #endif
+    aws_thread_id_t thread_id;
 };
 
 AWS_EXTERN_C_BEGIN
@@ -86,7 +93,7 @@ int aws_thread_launch(
  * Gets the id of thread
  */
 AWS_COMMON_API
-uint64_t aws_thread_get_id(struct aws_thread *thread);
+aws_thread_id_t aws_thread_get_id(struct aws_thread *thread);
 
 /**
  * Gets the detach state of the thread. For example, is it safe to call join on
@@ -110,10 +117,16 @@ AWS_COMMON_API
 void aws_thread_clean_up(struct aws_thread *thread);
 
 /**
- * returns the thread id of the calling thread.
+ * Returns the thread id of the calling thread.
+ */
+AWS_COMMON_API
+aws_thread_id_t aws_thread_current_thread_id(void);
+
+/**
+ * Compare thread ids.
  */
 AWS_COMMON_API
-uint64_t aws_thread_current_thread_id(void);
+bool aws_thread_thread_id_equal(aws_thread_id_t t1, aws_thread_id_t t2);
 
 /**
  * Sleeps the current thread by nanos.

source/log_formatter.c

@@ -51,6 +51,13 @@ static size_t s_advance_and_clamp_index(size_t current_index, int amount, size_t
 
     return next_index;
 }
+
+/* Thread-local string representation of current thread id */
+AWS_THREAD_LOCAL struct {
+    bool is_valid;
+    char repr[AWS_THREAD_ID_T_REPR_BUFSZ];
+} tl_logging_thread_id = {.is_valid = false};
+
 int aws_format_standard_log_line(struct aws_logging_standard_formatting_data *formatting_data, va_list args) {
     size_t current_index = 0;
 
@@ -109,16 +116,21 @@ int aws_format_standard_log_line(struct aws_logging_standard_formatting_data *fo
         /*
          * Add thread id and user content separator (" - ")
          */
-        uint64_t current_thread_id = aws_thread_current_thread_id();
+        if (!tl_logging_thread_id.is_valid) {
+            aws_thread_id_t current_thread_id = aws_thread_current_thread_id();
+            if (aws_thread_id_t_to_string(current_thread_id, tl_logging_thread_id.repr, AWS_THREAD_ID_T_REPR_BUFSZ)) {
+                return AWS_OP_ERR;
+            }
+            tl_logging_thread_id.is_valid = true;
+        }
         int thread_id_written = snprintf(
             formatting_data->log_line_buffer + current_index,
             fake_total_length - current_index,
-            "] [%" PRIu64 "] ",
-            current_thread_id);
+            "] [%s] ",
+            tl_logging_thread_id.repr);
         if (thread_id_written < 0) {
             return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
         }
-
         current_index = s_advance_and_clamp_index(current_index, thread_id_written, fake_total_length);
     }
 

source/logging.c

@@ -291,6 +291,25 @@ int aws_log_level_to_string(enum aws_log_level log_level, const char **level_str
     return AWS_OP_SUCCESS;
 }
 
+int aws_thread_id_t_to_string(aws_thread_id_t thread_id, char *buffer, size_t bufsz) {
+    AWS_ERROR_PRECONDITION(AWS_THREAD_ID_T_REPR_BUFSZ == bufsz);
+    AWS_ERROR_PRECONDITION(buffer && AWS_MEM_IS_WRITABLE(buffer, bufsz));
+    size_t current_index = 0;
+    unsigned char *bytes = (unsigned char *)&thread_id;
+    for (size_t i = sizeof(aws_thread_id_t); i != 0; --i) {
+        unsigned char c = bytes[i - 1];
+        int written = snprintf(buffer + current_index, bufsz - current_index, "%02x", c);
+        if (written < 0) {
+            return AWS_OP_ERR;
+        }
+        current_index += written;
+        if (bufsz <= current_index) {
+            return AWS_OP_ERR;
+        }
+    }
+    return AWS_OP_SUCCESS;
+}
+
 #ifndef AWS_MAX_LOG_SUBJECT_SLOTS
 #    define AWS_MAX_LOG_SUBJECT_SLOTS 16u
 #endif

source/posix/thread.c

@@ -97,9 +97,7 @@ void aws_thread_call_once(aws_thread_once *flag, void (*call_once)(void *), void
 }
 
 int aws_thread_init(struct aws_thread *thread, struct aws_allocator *allocator) {
-    thread->allocator = allocator;
-    thread->thread_id = 0;
-    thread->detach_state = AWS_THREAD_NOT_CREATED;
+    *thread = (struct aws_thread){.allocator = allocator, .detach_state = AWS_THREAD_NOT_CREATED};
 
     return AWS_OP_SUCCESS;
 }
@@ -176,8 +174,8 @@ int aws_thread_launch(
     return AWS_OP_SUCCESS;
 }
 
-uint64_t aws_thread_get_id(struct aws_thread *thread) {
-    return (uintptr_t)thread->thread_id;
+aws_thread_id_t aws_thread_get_id(struct aws_thread *thread) {
+    return thread->thread_id;
 }
 
 enum aws_thread_detach_state aws_thread_get_detach_state(struct aws_thread *thread) {
@@ -206,8 +204,12 @@ int aws_thread_join(struct aws_thread *thread) {
     return AWS_OP_SUCCESS;
 }
 
-uint64_t aws_thread_current_thread_id(void) {
-    return (uintptr_t)pthread_self();
+aws_thread_id_t aws_thread_current_thread_id(void) {
+    return pthread_self();
+}
+
+bool aws_thread_thread_id_equal(aws_thread_id_t t1, aws_thread_id_t t2) {
+    return pthread_equal(t1, t2) != 0;
 }
 
 void aws_thread_current_sleep(uint64_t nanos) {

source/windows/thread.c

@@ -125,7 +125,7 @@ int aws_thread_launch(
     return AWS_OP_SUCCESS;
 }
 
-uint64_t aws_thread_get_id(struct aws_thread *thread) {
+aws_thread_id_t aws_thread_get_id(struct aws_thread *thread) {
     return thread->thread_id;
 }
 
@@ -147,8 +147,12 @@ void aws_thread_clean_up(struct aws_thread *thread) {
     thread->thread_handle = 0;
 }
 
-uint64_t aws_thread_current_thread_id(void) {
-    return (uint64_t)GetCurrentThreadId();
+aws_thread_id_t aws_thread_current_thread_id(void) {
+    return GetCurrentThreadId();
+}
+
+bool aws_thread_thread_id_equal(aws_thread_id_t t1, aws_thread_id_t t2) {
+    return t1 == t2;
 }
 
 void aws_thread_current_sleep(uint64_t nanos) {

tests/error_test.c

@@ -359,20 +359,20 @@ static int s_unknown_error_code_range_too_large_test_fn(struct aws_allocator *al
 struct error_thread_test_data {
     int thread_1_code;
     int thread_1_get_last_code;
-    uint64_t thread_1_id;
+    aws_thread_id_t thread_1_id;
     int thread_1_encountered_count;
     int thread_2_code;
     int thread_2_get_last_code;
     int thread_2_encountered_count;
-    uint64_t thread_2_id;
+    aws_thread_id_t thread_2_id;
 };
 
 static void s_error_thread_test_thread_local_cb(int err, void *ctx) {
     struct error_thread_test_data *cb_data = (struct error_thread_test_data *)ctx;
 
-    uint64_t thread_id = aws_thread_current_thread_id();
+    aws_thread_id_t thread_id = aws_thread_current_thread_id();
 
-    if (thread_id == cb_data->thread_1_id) {
+    if (aws_thread_thread_id_equal(thread_id, cb_data->thread_1_id)) {
         cb_data->thread_1_code = err;
         cb_data->thread_1_get_last_code = aws_last_error();
         cb_data->thread_1_encountered_count += 1;

tests/logging/log_formatter_test.c

@@ -122,14 +122,20 @@ int do_default_log_formatter_test(
 
     char *thread_id_end = strstr(thread_id_start, "]");
     ASSERT_TRUE(thread_id_end != NULL, "Could not find end of thread id in output line \"%s\"", buffer);
-    char *thread_id_end_copy = thread_id_end;
 
-    uint64_t current_thread_id = aws_thread_current_thread_id();
-    uint64_t logged_id = strtoumax(thread_id_start, &thread_id_end_copy, 10);
-    ASSERT_TRUE(
-        logged_id == current_thread_id,
-        "Expected logged thread id to be %" PRIu64 " but it was actually %" PRIu64 "",
-        current_thread_id,
+    ASSERT_TRUE((thread_id_end - thread_id_start + 1) == AWS_THREAD_ID_T_REPR_BUFSZ, "Unexpected thread id length");
+    aws_thread_id_t current_thread_id = aws_thread_current_thread_id();
+    char repr[AWS_THREAD_ID_T_REPR_BUFSZ];
+    ASSERT_SUCCESS(
+        aws_thread_id_t_to_string(current_thread_id, repr, AWS_THREAD_ID_T_REPR_BUFSZ),
+        "Could not convert aws_thread_id_t to string repr");
+    char logged_id[AWS_THREAD_ID_T_REPR_BUFSZ];
+    memcpy(logged_id, thread_id_start, AWS_THREAD_ID_T_REPR_BUFSZ - 1);
+    logged_id[AWS_THREAD_ID_T_REPR_BUFSZ - 1] = '\0';
+    ASSERT_SUCCESS(
+        strncmp(repr, logged_id, AWS_THREAD_ID_T_REPR_BUFSZ),
+        "Expected logged thread id to be \"%s\" but it was actually \"%s\"",
+        repr,
         logged_id);
 
     /*

tests/thread_test.c

@@ -18,7 +18,7 @@
 #include <aws/testing/aws_test_harness.h>
 
 struct thread_test_data {
-    uint64_t thread_id;
+    aws_thread_id_t thread_id;
 };
 
 static void s_thread_fn(void *arg) {
@@ -29,7 +29,6 @@ static void s_thread_fn(void *arg) {
 static int s_test_thread_creation_join_fn(struct aws_allocator *allocator, void *ctx) {
     (void)ctx;
     struct thread_test_data test_data;
-    test_data.thread_id = 0;
 
     struct aws_thread thread;
     aws_thread_init(&thread, allocator);
@@ -38,9 +37,8 @@ static int s_test_thread_creation_join_fn(struct aws_allocator *allocator, void
     ASSERT_INT_EQUALS(
         AWS_THREAD_JOINABLE, aws_thread_get_detach_state(&thread), "thread state should have returned JOINABLE");
     ASSERT_SUCCESS(aws_thread_join(&thread), "thread join failed");
-    ASSERT_INT_EQUALS(
-        test_data.thread_id,
-        aws_thread_get_id(&thread),
+    ASSERT_TRUE(
+        aws_thread_thread_id_equal(test_data.thread_id, aws_thread_get_id(&thread)),
         "get_thread_id should have returned the same id as the thread calling current_thread_id");
     ASSERT_INT_EQUALS(
         AWS_THREAD_JOIN_COMPLETED,