kgiusti commented on a change in pull request #1301:
URL: https://github.com/apache/qpid-dispatch/pull/1301#discussion_r675802653



##########
File path: src/adaptors/http2/http2_adaptor.c
##########
@@ -2496,10 +2657,21 @@ static void handle_connection_event(pn_event_t *e, 
qd_server_t *qd_server, void
     }
     case PN_RAW_CONNECTION_WAKE: {
         qd_log(log, QD_LOG_TRACE, "[C%"PRIu64"] PN_RAW_CONNECTION_WAKE 
Wake-up", conn->conn_id);
+        if (IS_ATOMIC_FLAG_SET(&conn->q2_restart)) {

Review comment:
       Precisely - SET and CLEAR operations must return the *previous* value, 
so if (CLEAR_ATOMIC_FLAG()) is true you know it went from True->False ... 
atomically.
   
   Imagine two threads running this code concurrently, one sees 
IS_ATOMIC_FLAG_SET() as true, enters the IF statement, then immediately swaps 
out.  The second thread also sees IS_ATOMIC_FLAG_SET() as true and also enters 
the IF block.   With an atomic CLEAR->return old value it is guaranteed that 
only one thread will ever see the return value of True and enter the IF.
   
   That said, it won't make a difference since only one thread is running the 
event handler so you're pretty much guaranteed that no other thread will be 
making the same IS_XXX CLEAR__XX calls concurrently.
   
   But why do two atomic operations (both of which cause a cache flush) when 
one call does both?




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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to