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.

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

[FIXED] Missing connection status check causing unexpected delay during connection close #654

Merged
merged 1 commit into from
May 15, 2023

Conversation

kolomenkin
Copy link
Contributor

It happens when connection close was started and it signaled conditional variable before waiting for the same variable was started in reconnect thread.

The code example to reproduce the problem:

int main()
{
  printf("Begin...\n");
  natsConnection * connection = nullptr;
  natsOptions * options = nullptr;
  constexpr auto onConnectHandler = [](natsConnection *, void *)
  {
  };

  natsOptions_Create(&options);
  natsOptions_SetRetryOnFailedConnect(options, true, onConnectHandler, nullptr);
  natsOptions_SetURL(options, "nats://localhost:54321");

  const auto ts1 = nats_Now();
  const natsStatus err = natsConnection_Connect(&connection, options);
  const auto ts2 = nats_Now();

  if (err == NATS_NOT_YET_CONNECTED || err == NATS_OK)
  {
    natsConnection_Close(connection);
  }
  const auto ts3 = nats_Now();

  printf("connection err code: %s\n", natsStatus_GetText(err));
  printf("Connect took: %lld ms\n", (ts2 - ts1));
  printf("Connection close took: %lld ms\n", (ts3 - ts2));
  return 0;
}

Program output:

Begin...
connection err code: Not Yet Connected
Connect took: 2017 ms
Connection close took: 2057 ms

Here we are connecting to a NATS server which is offline. NATS_NOT_YET_CONNECTED status is returned.
We have 2 seconds before next connection try.
I expect connection to close very fast.
But closing freezes for these 2 seconds.

It happens when connection close was started and it signaled
conditional variable before waiting for the same variable
was started in reconnect thread.

The code example to reproduce the problem:
```c++
int main()
{
  printf("Begin...\n");
  natsConnection * connection = nullptr;
  natsOptions * options = nullptr;
  constexpr auto onConnectHandler = [](natsConnection *, void *)
  {
  };

  natsOptions_Create(&options);
  natsOptions_SetRetryOnFailedConnect(options, true, onConnectHandler, nullptr);
  natsOptions_SetURL(options, "nats://localhost:54321");

  const auto ts1 = nats_Now();
  const natsStatus err = natsConnection_Connect(&connection, options);
  const auto ts2 = nats_Now();

  if (err == NATS_NOT_YET_CONNECTED || err == NATS_OK)
  {
    natsConnection_Close(connection);
  }
  const auto ts3 = nats_Now();

  printf("connection err code: %s\n", natsStatus_GetText(err));
  printf("Connect took: %lld ms\n", (ts2 - ts1));
  printf("Connection close took: %lld ms\n", (ts3 - ts2));
  return 0;
}
```

Program output:
```
Begin...
connection err code: Not Yet Connected
Connect took: 2017 ms
Connection close took: 2057 ms
```

Here we are connecting to a NATS server which is offline.
NATS_NOT_YET_CONNECTED status is returned.
We have 2 seconds before next connection try.
I expect connection to close very fast.
But closing freezes for these 2 seconds.
@kozlovic kozlovic changed the title fix race condition causing unexpected delay during connection close [FIXED] Missing connection status check causing unexpected delay during connection close May 15, 2023
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for the contribution. I will add a test after merging this.

@kozlovic kozlovic merged commit 8c02dcc into nats-io:main May 15, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants