[GitHub] [qpid-dispatch] ted-ross commented on a change in pull request #817: DISPATCH-1738 - Zero out the router_id references on core and router …

[GitBox <gi...@apache.org>]

ted-ross commented on a change in pull request #817:
URL: https://github.com/apache/qpid-dispatch/pull/817#discussion_r467148808



##########
File path: src/router_core/modules/mobile_sync/mobile.c
##########
@@ -206,6 +206,15 @@ static void qcm_mobile_sync_compose_diff_hint_list(qdrm_mobile_sync_t *msync, qd
 
 static qd_message_t *qcm_mobile_sync_compose_differential_mau(qdrm_mobile_sync_t *msync, const char *address)
 {
+    //
+    // DISPATCH-1738: If the router is shutting down, the qd_dispatch_free() in dispatch.c frees the qd->router_id and
+    // sets the core->router_id to zero. When the core is shutting down, the core->router_id will be zero
+    // and we don't want to proceed with this function. We return here in order to prevent the use after free error on
+    // msync->core->router_id down below in the function.
+    //
+    if (! msync->core->router_id)
+        return 0;
+

Review comment:
       Would it not be better to ensure that the core thread is shut down before freeing the router_id?  If the core thread is running while basic state is being freed, there must be many other potential issues in the code.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org