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

Look into paged result search locking #6518

Open
mreynolds389 opened this issue Jan 20, 2025 · 2 comments
Open

Look into paged result search locking #6518

mreynolds389 opened this issue Jan 20, 2025 · 2 comments
Labels
needs triage The issue will be triaged during scrum

Comments

@mreynolds389
Copy link
Contributor

Issue Description

Currently we take paged result search lock in the polling threads. If a PR search result is being sent back and gets blocked it will also block the polling thread.

This blocking can only happen with concurrent PR searches on the same connection (like what SSSD does).

@mreynolds389 mreynolds389 added the needs triage The issue will be triaged during scrum label Jan 20, 2025
@mreynolds389
Copy link
Contributor Author

Not sure if this is safe, but this is a possible solution:

diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c
index dda92e1f2..59e52842b 100644
--- a/ldap/servers/slapd/opshared.c
+++ b/ldap/servers/slapd/opshared.c
@@ -749,7 +749,10 @@ op_shared_search(Slapi_PBlock *pb, int send_result)
                     goto free_and_return;
                 } else {
                     slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_SET, pr_search_result);
+
+                    pthread_mutex_unlock(pagedresults_mutex);
                     rc = send_results_ext(pb, 1, &pnentries, pagesize, &pr_stat);
+                    pthread_mutex_lock(pagedresults_mutex);
 
                     /* search result could be reset in the backend/dse */
                     slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr);

@progier389
Copy link
Contributor

I do not think it is safe:
I am afraid that pnentries data can be freed if we do not own the lock
(If I remember rightly the paged search data removal is based on a timeout (that can fire while waiting on sent result))
I think that we will rather need to be more clever and generate a list of the message to send (instead of sending directly the data)
then release the lock
iterate on the list to send data
free the list
acquire back the lock

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage The issue will be triaged during scrum
Projects
None yet
Development

No branches or pull requests

2 participants