[GitHub] [activemq-nms-amqp] cjwmorgan-sol commented on issue #45: AMQNET-624: Fix failover issue when broker sends open frame and shortly after close frame

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

cjwmorgan-sol commented on issue #45: AMQNET-624: Fix failover issue when broker sends open frame and shortly after close frame
URL: https://github.com/apache/activemq-nms-amqp/pull/45#issuecomment-547534912
 
 
   Looks good and tests pass. 
   
   The only thing is running TriggerReconnectionAttempt outside of lock(SyncRoot) by using Task.Run(). Does this occur? (I'm not too strong in using async task pattern with locking) The FailoverProvider underlying provider seems to be manipulated safely only under the lock(SyncRoot) however I believe there is no lock(SyncRoot) in InitializeNewConnection allowing old providers to be left opened and live. I was able to add a log in InitializeNewConnection to check if an old provider was still there from the FailoverProvider.provider field and I was able to see logs FailoverProvider.provider having an old value during InitializeNewConnection. 
   
   @Havret Should that be possible?

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