Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix bug where reconnection failed if client had been previously closed #3196

Merged
merged 1 commit into from
Mar 27, 2023
Merged

Fix bug where reconnection failed if client had been previously closed #3196

merged 1 commit into from
Mar 27, 2023

Conversation

abhipsaMisra
Copy link
Member

There are two updates that this PR brings in:

  • the code to open a client was duplicated in two places:
    • client is opened explicitly
    • client is opened as a result of a reconnection attempt
      This PR merges both of those flows
  • if a client was opened after it had been previously closed, any reconnection attempt as a result of transport disconnection would fail. This is because the cancellation token source influencing subsequent reconnection attempt was canceled when CloseAsync() was called, and it was never reinitialized.

abhipsaMisra
abhipsaMisra commented Mar 27, 2023
View reviewed changes
iothub/device/src/Pipeline/RetryDelegatingHandler.cs
{
var connectionStatusInfo = new ConnectionStatusInfo();

_handleDisconnectCts ??= new CancellationTokenSource();
Copy link
Member Author

@abhipsaMisra abhipsaMisra Mar 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cancellation token guiding reconnection attempts is initialized when the client is opened. This cancellation token source is canceled, disposed and set to null when the client is closed, it is disposed, or the connection attempt fails.

@abhipsaMisra
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@abhipsaMisra abhipsaMisra merged commit 2b8233e into previews/v2 Mar 27, 2023
@abhipsaMisra abhipsaMisra deleted the abmisr/closeOpen branch March 27, 2023 21:45
@brycewang-microsoft brycewang-microsoft mentioned this pull request Apr 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brycewang-microsoft brycewang-microsoft brycewang-microsoft approved these changes

@drwill-ms drwill-ms Awaiting requested review from drwill-ms

@timtay-microsoft timtay-microsoft Awaiting requested review from timtay-microsoft timtay-microsoft is a code owner

@andyk-ms andyk-ms Awaiting requested review from andyk-ms andyk-ms is a code owner

@tmahmood-microsoft tmahmood-microsoft Awaiting requested review from tmahmood-microsoft tmahmood-microsoft is a code owner

@patilsnr patilsnr Awaiting requested review from patilsnr patilsnr is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@abhipsaMisra @brycewang-microsoft