fix(iot-dev): Fix bug where multiplexed client had more session handlers than expected#1341
Conversation
…dlers than expected If a multiplexing client tried to open, failed due to a network issue or something similar, and then later tried to open again, the session handler list would contain 2 session handlers for each multiplexed device instead of the expected 1. The issue was that the catch logic within the open call didn't clear the session handler list in the event of a network issue or interruption.
timtay-microsoft
requested review from
abhipsaMisra,
azabbasi,
drwill-ms and
jamdavi
as code owners
| if (existingAmqpsSessionHandler.getDeviceId().equals(deviceClientConfig.getDeviceId())) | ||
| { | ||
| amqpsSessionHandler = existingAmqpsSessionHandler; | ||
| break; |
There was a problem hiding this comment.
This session handler was retrieved from the local state, but later added to the same list it was retrieved from for some reason. Now we just return it once it is found. No need to add a duplicate session handler to the list of session handlers
| else | ||
| { | ||
| // Since a session is being added for the reconnecting device, it can be removed from the reconnectingDeviceSessionHandlers collection | ||
| this.reconnectingDeviceSessionHandlers.remove(amqpsSessionHandler); |
There was a problem hiding this comment.
This list is of multiplexed devices that are temporarily unregistered for reconnection purposes. Since the addSessionHandler call is used to re-add the unregistered device, now is the appropriate time to remove the device from this list.
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
|
|
||
| private void clearLocalState() | ||
| { | ||
| this.sessionHandlers.clear(); |
There was a problem hiding this comment.
Would there be any need to nullcheck anything in here? Just seeing it being called in the catch so it might give an unexpected NRE.
There was a problem hiding this comment.
Nope. Both of these queues are final, so they are instantiated once and never set to null
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
2 participants
If a multiplexing client tried to open, failed due to a network issue or something similar, and then later tried to open again, the session handler list would contain 2 session handlers for each multiplexed device instead of the expected 1. The issue was that the catch logic within the open call didn't clear the session handler list in the event of a network issue or interruption.
#1336