andykimpe / rpms / 389-ds-base

Forked from rpms/389-ds-base 5 months ago
Clone
Blob Blame History Raw
From f50e991828321e3e19ac704f0b2e0968a0c6704e Mon Sep 17 00:00:00 2001
From: Noriko Hosoi <nhosoi@redhat.com>
Date: Thu, 5 Nov 2015 10:44:08 -0800
Subject: [PATCH 365/365] Ticket #48338 - SimplePagedResults -- abandon could
 happen between the abandon check and sending results

Description: An abandon request for a SimplePagedResults request could
happened between the abandon check and the code for sending the search
results.  The abandon frees the search results although sending result
code still refers it.

Fix description: The code (from getting search results through sending
the search results) in op_shared_search is protected by c_mutex locking.

https://fedorahosted.org/389/ticket/48338

Reviewed by rmeggins@redhat.com (Thank you, Rich!!)

(cherry picked from commit 390b8bd9076e8976facc0858e60985d6b4fac05c)
(cherry picked from commit 8f49d33d30fade7b579062414250a0ddb1a66c62)
(cherry picked from commit a7a8b2d3cb8452cc629ec5228bea6571d72e6538)
(cherry picked from commit d5ccc06cefa6f423d80c6cc0e527b57c116b0cb8)
---
 ldap/servers/slapd/opshared.c     | 13 ++++++++-----
 ldap/servers/slapd/pagedresults.c | 28 ++++++++++++++++++----------
 ldap/servers/slapd/proto-slap.h   |  5 ++---
 3 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c
index 31d41e8..391d1c3 100644
--- a/ldap/servers/slapd/opshared.c
+++ b/ldap/servers/slapd/opshared.c
@@ -530,7 +530,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
                       be = be_list[index];
                   }
               }
-              pr_search_result = pagedresults_get_search_result(pb->pb_conn, operation, pr_idx);
+              pr_search_result = pagedresults_get_search_result(pb->pb_conn, operation, 0/*not locked*/, pr_idx);
               estimate = pagedresults_get_search_result_set_size_estimate(pb->pb_conn, operation, pr_idx);
               if (pagedresults_get_unindexed(pb->pb_conn, operation, pr_idx)) {
                   opnote |= SLAPI_OP_NOTE_UNINDEXED;
@@ -700,13 +700,15 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
        * In async paged result case, the search result might be released
        * by other theads.  We need to double check it in the locked region.
        */
-      pr_search_result = pagedresults_get_search_result(pb->pb_conn, operation, pr_idx);
+      PR_Lock(pb->pb_conn->c_mutex);
+      pr_search_result = pagedresults_get_search_result(pb->pb_conn, operation, 1/*locked*/, pr_idx);
       if (pr_search_result) {
-        if (pagedresults_is_abandoned_or_notavailable(pb->pb_conn, pr_idx)) {
+        if (pagedresults_is_abandoned_or_notavailable(pb->pb_conn, 1/*locked*/, pr_idx)) {
           pagedresults_unlock(pb->pb_conn, pr_idx);
           /* Previous operation was abandoned and the simplepaged object is not in use. */
           send_ldap_result(pb, 0, NULL, "Simple Paged Results Search abandoned", 0, NULL);
           rc = LDAP_SUCCESS;
+          PR_Unlock(pb->pb_conn->c_mutex);
           goto free_and_return;
         } else {
           slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, pr_search_result );
@@ -714,7 +716,8 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
 
           /* search result could be reset in the backend/dse */
           slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr);
-          pagedresults_set_search_result(pb->pb_conn, operation, sr, 0, pr_idx);
+          pagedresults_set_search_result(pb->pb_conn, operation, sr, 1/*locked*/, pr_idx);
+          PR_Unlock(pb->pb_conn->c_mutex);
         }
       } else {
         pr_stat = PAGEDRESULTS_SEARCH_END;
@@ -745,7 +748,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
       if (PAGEDRESULTS_SEARCH_END == pr_stat) {
         pagedresults_lock(pb->pb_conn, pr_idx);
         slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_SET, NULL);
-        if (!pagedresults_is_abandoned_or_notavailable(pb->pb_conn, pr_idx)) {
+        if (!pagedresults_is_abandoned_or_notavailable(pb->pb_conn, 0/*not locked*/, pr_idx)) {
           pagedresults_free_one(pb->pb_conn, operation, pr_idx);
         }
         pagedresults_unlock(pb->pb_conn, pr_idx);
diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c
index 434e48d..c1947f1 100644
--- a/ldap/servers/slapd/pagedresults.c
+++ b/ldap/servers/slapd/pagedresults.c
@@ -408,20 +408,25 @@ pagedresults_set_current_be(Connection *conn, Slapi_Backend *be, int index, int
 }
 
 void *
-pagedresults_get_search_result(Connection *conn, Operation *op, int index)
+pagedresults_get_search_result(Connection *conn, Operation *op, int locked, int index)
 {
     void *sr = NULL;
     if (!op_is_pagedresults(op)) {
         return sr; /* noop */
     }
-    LDAPDebug1Arg(LDAP_DEBUG_TRACE,
-                  "--> pagedresults_get_search_result: idx=%d\n", index);
+    LDAPDebug2Args(LDAP_DEBUG_TRACE,
+                   "--> pagedresults_get_search_result(%s): idx=%d\n",
+                   locked?"locked":"not locked", index);
     if (conn && (index > -1)) {
-        PR_Lock(conn->c_mutex);
+        if (!locked) {
+            PR_Lock(conn->c_mutex);
+        }
         if (index < conn->c_pagedresults.prl_maxlen) {
             sr = conn->c_pagedresults.prl_list[index].pr_search_result_set;
         }
-        PR_Unlock(conn->c_mutex);
+        if (!locked) {
+            PR_Unlock(conn->c_mutex);
+        }
     }
     LDAPDebug1Arg(LDAP_DEBUG_TRACE,
                   "<-- pagedresults_get_search_result: %p\n", sr);
@@ -429,8 +434,7 @@ pagedresults_get_search_result(Connection *conn, Operation *op, int index)
 }
 
 int
-pagedresults_set_search_result(Connection *conn, Operation *op, void *sr, 
-                               int locked, int index)
+pagedresults_set_search_result(Connection *conn, Operation *op, void *sr, int locked, int index)
 {
     int rc = -1;
     if (!op_is_pagedresults(op)) {
@@ -1016,15 +1020,19 @@ pagedresults_unlock( Connection *conn, int index )
 }
 
 int
-pagedresults_is_abandoned_or_notavailable( Connection *conn, int index )
+pagedresults_is_abandoned_or_notavailable(Connection *conn, int locked, int index)
 {
     PagedResults *prp;
     if (!conn || (index < 0) || (index >= conn->c_pagedresults.prl_maxlen)) {
         return 1; /* not abandoned, but do not want to proceed paged results op. */
     }
-    PR_Lock(conn->c_mutex);
+    if (!locked) {
+        PR_Lock(conn->c_mutex);
+    }
     prp = conn->c_pagedresults.prl_list + index;
-    PR_Unlock(conn->c_mutex);
+    if (!locked) {
+        PR_Unlock(conn->c_mutex);
+    }
     return prp->pr_flags & CONN_FLAG_PAGEDRESULTS_ABANDONED;
 }
 
diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h
index 8f86eb7..decc29e 100644
--- a/ldap/servers/slapd/proto-slap.h
+++ b/ldap/servers/slapd/proto-slap.h
@@ -1438,8 +1438,7 @@ void pagedresults_set_response_control(Slapi_PBlock *pb, int iscritical,
                                        int curr_search_count, int index);
 Slapi_Backend *pagedresults_get_current_be(Connection *conn, int index);
 int pagedresults_set_current_be(Connection *conn, Slapi_Backend *be, int index, int nolock);
-void *pagedresults_get_search_result(Connection *conn, Operation *op,
-                                     int index);
+void *pagedresults_get_search_result(Connection *conn, Operation *op, int locked, int index);
 int pagedresults_set_search_result(Connection *conn, Operation *op, void *sr, 
                                    int locked, int index);
 int pagedresults_get_search_result_count(Connection *conn, Operation *op,
@@ -1480,7 +1479,7 @@ int pagedresults_cleanup_all(Connection *conn, int needlock);
 void op_set_pagedresults(Operation *op);
 void pagedresults_lock(Connection *conn, int index);
 void pagedresults_unlock(Connection *conn, int index);
-int pagedresults_is_abandoned_or_notavailable(Connection *conn, int index);
+int pagedresults_is_abandoned_or_notavailable(Connection *conn, int locked, int index);
 int pagedresults_set_search_result_pb(Slapi_PBlock *pb, void *sr, int locked);
 
 /*
-- 
2.4.3