[GitHub] [qpid-dispatch] ChugR commented on a change in pull request #565: DISPATCH-1417 - First attempt at fixing router crash by introducing a…

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

ChugR commented on a change in pull request #565: DISPATCH-1417 - First attempt at fixing router crash by introducing a…
URL: https://github.com/apache/qpid-dispatch/pull/565#discussion_r323941648
 
 

 ##########
 File path: src/server.c
 ##########
 @@ -1011,7 +1012,9 @@ static void *thread_run(void *arg)
             /* Free the connection after all other processing is complete */
             if (qd_conn && pn_event_type(e) == PN_TRANSPORT_CLOSED) {
                 pn_connection_set_context(pn_conn, NULL);
+                sys_mutex_lock(qd_server->conn_activation_lock);
 
 Review comment:
   It looks like a regular worker thread will take out CONN_ACTIVATION_LOCK to free the connection. qd_connection_free takes out the SERVER_LOCK and then takes out the PYTHON_LOCK to account for policy-counted connections.
   Meanwhile, the core thread will take out CONN_ACTIVATION_LOCK while it processes qd_server_activate.
   Although it didn't fail in my DISPATCH-1417 tests it seems like this lock cycle could be a real problem for the core thread. Core never was blocked by SERVER and PYTHON locks before. I don't suspect a deadlock but the core thread will suffer large waits. 

----------------------------------------------------------------
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


With regards,
Apache Git Services

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