clean up comments

include/aws/io/socket.h

@@ -212,9 +212,10 @@ AWS_IO_API void aws_socket_clean_up(struct aws_socket *socket);
  * In TCP, LOCAL and VSOCK this function will not block. If the return value is successful, then you must wait on the
  * `on_connection_result()` callback to be invoked before using the socket.
  *
- * The function will failed with error if the endpoint is invalid. Except for LOCAL with AWS_USE_DISPATCH_QUEUE (with
- * Apple Network Framework enabled). In Apple network framework, we would not know if the local endpoint is valid until
- * we have the connection state back, Make sure to validate in `on_connection_result` for this case.
+ * The function will failed with error if the endpoint is invalid. Except for Apple Network Framework (with
+ * AWS_USE_DISPATCH_QUEUE enabled). In Apple network framework, as connect is an async api, we would not know if the
+ * local endpoint is valid until we have the connection state returned in callback. The error will returned in
+ * `on_connection_result` callback
  *
  * If an event_loop is provided for UDP sockets, a notification will be sent on
  * on_connection_result in the event-loop's thread. Upon completion, the socket will already be assigned

source/channel.c

@@ -645,8 +645,11 @@ void aws_channel_schedule_task_future(
 }
 
 bool aws_channel_thread_is_callers_thread(struct aws_channel *channel) {
-    if (channel && channel->loop)
+    // As aws_event_loop is not ref-counted, it is possible that the socket read/write callback get triggered after 
+    // the event loop is released, ignore the function call here. 
+    if (channel && channel->loop){
         return aws_event_loop_thread_is_callers_thread(channel->loop);
+    }
     return true;
 }
 

source/darwin/dispatch_queue_event_loop.c

@@ -54,7 +54,7 @@ struct scheduled_service_entry {
     uint64_t timestamp;
     struct aws_linked_list_node node;
     struct aws_event_loop *loop; // might eventually need to be ref-counted for cleanup?
-    bool cancel;
+    bool cancel;                 // The entry will be canceled if the event loop is destroyed.
 };
 
 struct scheduled_service_entry *scheduled_service_entry_new(struct aws_event_loop *loop, uint64_t timestamp) {
@@ -201,6 +201,7 @@ static void s_destroy(struct aws_event_loop *event_loop) {
     AWS_LOGF_TRACE(AWS_LS_IO_EVENT_LOOP, "id=%p: Destroying Dispatch Queue Event Loop", (void *)event_loop);
     struct dispatch_loop *dispatch_loop = event_loop->impl_data;
 
+    // Avoid double destroy
     if (dispatch_loop->is_destroying) {
         return;
     }
@@ -231,8 +232,9 @@ static void s_destroy(struct aws_event_loop *event_loop) {
       }
 
       aws_mutex_lock(&dispatch_loop->synced_data.lock);
-      // The entry in the scheduled_services are all pushed to dispatch loop as the function context.
-      // Apple does not allow NULL context here, do not destroy the entry until the block run.
+      // The entries in the scheduled_services are already put on the apple dispatch queue. It would be a bad memory
+      // access if we destroy the entries here. We instead setting a cancel flag to cancel the task when the
+      // dispatch_queue execute the entry.
       struct aws_linked_list_node *iter = NULL;
       for (iter = aws_linked_list_begin(&dispatch_loop->synced_data.scheduling_state.scheduled_services);
            iter != aws_linked_list_end(&dispatch_loop->synced_data.scheduling_state.scheduled_services);

source/darwin/nw_socket.c

@@ -434,11 +434,11 @@ static void s_process_readable_task(struct aws_task *task, void *arg, enum aws_t
     if (status != AWS_TASK_STATUS_CANCELED) {
         aws_mutex_lock(&nw_socket->synced_data.lock);
         struct aws_socket *socket = nw_socket->synced_data.base_socket;
-        if (readable_args->error_code == AWS_IO_SOCKET_CLOSED) {
-            aws_socket_close(socket);
-        }
 
         if (socket && nw_socket->on_readable) {
+            if (readable_args->error_code == AWS_IO_SOCKET_CLOSED) {
+                aws_socket_close(socket);
+            }
             if (readable_args->data) {
                 struct read_queue_node *node = aws_mem_calloc(nw_socket->allocator, 1, sizeof(struct read_queue_node));
                 node->allocator = nw_socket->allocator;
@@ -1473,7 +1473,6 @@ static int s_socket_read_fn(struct aws_socket *socket, struct aws_byte_buf *read
         struct aws_linked_list_node *node = aws_linked_list_front(&nw_socket->read_queue);
         struct read_queue_node *read_node = AWS_CONTAINER_OF(node, struct read_queue_node, node);
 
-        AWS_LOGF_DEBUG(AWS_LS_IO_SOCKET, "id=%p, [DEBUG READ DATA] start processing node ", (void *)node);
         bool read_completed = dispatch_data_apply(
             read_node->received_data,
             (dispatch_data_applier_t) ^ (dispatch_data_t region, size_t offset, const void *buffer, size_t size) {
@@ -1492,7 +1491,6 @@ static int s_socket_read_fn(struct aws_socket *socket, struct aws_byte_buf *read
             });
 
         if (read_completed) {
-            AWS_LOGF_DEBUG(AWS_LS_IO_SOCKET, "id=%p, [DEBUG READ DATA] clean up the node", (void *)node);
             aws_linked_list_remove(node);
             s_clean_up_read_queue_node(read_node);
         }

tests/CMakeLists.txt

@@ -83,10 +83,12 @@ add_test_case(sock_write_cb_is_async)
 add_test_case(socket_validate_port)
 
 if(NOT AWS_USE_DISPATCH_QUEUE)
-# The read/write will always run a different thread for Apple Network Framework
-add_test_case(wrong_thread_read_write_fails)
 # Apple Network Framework does not support bind+connect
 add_test_case(udp_bind_connect_communication)
+# The read/write will always run a different thread for Apple Network Framework
+add_test_case(wrong_thread_read_write_fails)
+# Apple Network Framework would not validate the binding endpoint until we start the
+# listen. The test does not apply here.
 add_test_case(incoming_duplicate_tcp_bind_errors)
 endif()
 

tests/socket_test.c

@@ -29,9 +29,6 @@ static bool s_use_dispatch_queue = true;
 static bool s_use_dispatch_queue = false;
 #endif
 
-#define NANOS_PER_SEC ((uint64_t)AWS_TIMESTAMP_NANOS)
-#define TIMEOUT (10 * NANOS_PER_SEC)
-
 struct local_listener_args {
     struct aws_socket *incoming;
     struct aws_mutex *mutex;
@@ -642,7 +639,7 @@ static int s_test_socket_with_bind_to_interface(struct aws_allocator *allocator,
     (void)ctx;
     struct aws_socket_options options;
     AWS_ZERO_STRUCT(options);
-    options.connect_timeout_ms = 30000;
+    options.connect_timeout_ms = 3000;
     options.keepalive = true;
     options.keep_alive_interval_sec = 1000;
     options.keep_alive_timeout_sec = 60000;