Skip to content

Conversation

@tvernum
Copy link
Contributor

@tvernum tvernum commented Dec 1, 2021

LDAP SDK has a race condition where closing a connection while an
async search is still executing could lead to a Timer thread being
orphaned.
See: pingidentity/ldapsdk#120

This commit changes SearchGroupsResolverInMemoryTests so that it waits
for the pending async search to complete (or timeout) before
returning. This ensures that when the close the connection the Timer
thread is cancelled (and stays cancelled).

Resolves: #80305

LDAP SDK has a race condition where closing a connection while an
async search is still executing could lead to a Timer thread being
orphaned.
See: pingidentity/ldapsdk#120

This commit changes SearchGroupsResolverInMemoryTests so that it waits
for the pending async search to complete (or timeout) before
returning. This ensures that when the close the connection the Timer
thread is cancelled (and stays cancelled).
@tvernum tvernum added >test Issues or PRs that are addressing/adding tests :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0 auto-backport-and-merge v8.1.0 labels Dec 1, 2021
@tvernum tvernum requested a review from ywangd December 1, 2021 05:51
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Dec 1, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for digging into this issue. It's a great finding!

@tvernum tvernum added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Dec 2, 2021
@elasticsearchmachine elasticsearchmachine merged commit 591f551 into elastic:master Dec 2, 2021
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Dec 2, 2021
LDAP SDK has a race condition where closing a connection while an async
search is still executing could lead to a Timer thread being orphaned.
See: pingidentity/ldapsdk#120 This commit
changes SearchGroupsResolverInMemoryTests so that it waits for the
pending async search to complete (or timeout) before returning. This
ensures that when the close the connection the Timer thread is cancelled
(and stays cancelled). Resolves: elastic#80305
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.0

elasticsearchmachine pushed a commit that referenced this pull request Dec 2, 2021
LDAP SDK has a race condition where closing a connection while an async
search is still executing could lead to a Timer thread being orphaned.
See: pingidentity/ldapsdk#120 This commit
changes SearchGroupsResolverInMemoryTests so that it waits for the
pending async search to complete (or timeout) before returning. This
ensures that when the close the connection the Timer thread is cancelled
(and stays cancelled). Resolves: #80305
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team >test Issues or PRs that are addressing/adding tests v8.0.0-rc1 v8.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SearchGroupsResolverInMemoryTests leaking threads

5 participants