Blob Blame History Raw
From 577fa6ada5d860e5ed7291aff2e41938ca92793b Mon Sep 17 00:00:00 2001
From: Noriko Hosoi <nhosoi@redhat.com>
Date: Sun, 26 Apr 2015 14:46:44 -0700
Subject: [PATCH 64/72] Ticket #48146 - async simple paged results issue

Description: When the last page of the single paged results is returned,
the search results structure is freed in the simple paged results code
(in op_shared_search).  The search results structure is stashed in the
simple paged results object across the pages.  The free and the clean up
of the stashed address should have been atomic, but it was not.

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

Reviewed by mreynolds@redhat.com (Thank you, Mark!!)

(cherry picked from commit 947477f2e2367337a8c220d8c9d03a62bf1bbf1c)
(cherry picked from commit ec8801a69adbe2f502513fabcbb79bc5e38bf197)
---
 ldap/servers/slapd/opshared.c     | 18 +++++++-----------
 ldap/servers/slapd/pagedresults.c | 36 +++++++++++-------------------------
 2 files changed, 18 insertions(+), 36 deletions(-)

diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c
index ebd4fdf..e7cee8a 100644
--- a/ldap/servers/slapd/opshared.c
+++ b/ldap/servers/slapd/opshared.c
@@ -881,7 +881,10 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
             slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr);
             if (PAGEDRESULTS_SEARCH_END == pr_stat) {
               if (sr) { /* in case a left over sr is found, clean it up */
+                PR_Lock(pb->pb_conn->c_mutex);
+                pagedresults_set_search_result(pb->pb_conn, operation, NULL, 1, pr_idx);
                 be->be_search_results_release(&sr);
+                PR_Unlock(pb->pb_conn->c_mutex);
               }
               if (NULL == next_be) {
                 /* no more entries && no more backends */
@@ -897,17 +900,10 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
               slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate);
               pagedresults_lock(pb->pb_conn, pr_idx);
               if ((pagedresults_set_current_be(pb->pb_conn, be, pr_idx) < 0) ||
-                  (pagedresults_set_search_result(pb->pb_conn, operation,
-                                                  sr, 0, pr_idx) < 0) ||
-                  (pagedresults_set_search_result_count(pb->pb_conn, operation,
-                                                        curr_search_count,
-                                                        pr_idx) < 0) ||
-                  (pagedresults_set_search_result_set_size_estimate(pb->pb_conn,
-                                                                 operation,
-                                                                 estimate,
-                                                                 pr_idx) < 0) ||
-                  (pagedresults_set_with_sort(pb->pb_conn, operation,
-                                              with_sort, pr_idx) < 0)) {
+                  (pagedresults_set_search_result(pb->pb_conn, operation, sr, 0, pr_idx) < 0) ||
+                  (pagedresults_set_search_result_count(pb->pb_conn, operation, curr_search_count, pr_idx) < 0) ||
+                  (pagedresults_set_search_result_set_size_estimate(pb->pb_conn, operation, estimate, pr_idx) < 0) ||
+                  (pagedresults_set_with_sort(pb->pb_conn, operation, with_sort, pr_idx) < 0)) {
                 pagedresults_unlock(pb->pb_conn, pr_idx);
                 goto free_and_return;
               }
diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c
index d589708..e61c000 100644
--- a/ldap/servers/slapd/pagedresults.c
+++ b/ldap/servers/slapd/pagedresults.c
@@ -100,26 +100,20 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
     ber_free(ber, 1);
     if ( cookie.bv_len <= 0 ) {
         int i;
-        int maxlen;
         /* first time? */
-        maxlen = conn->c_pagedresults.prl_maxlen;
+        int maxlen = conn->c_pagedresults.prl_maxlen;
         if (conn->c_pagedresults.prl_count == maxlen) {
             if (0 == maxlen) { /* first time */
                 conn->c_pagedresults.prl_maxlen = 1;
-                conn->c_pagedresults.prl_list =
-                    (PagedResults *)slapi_ch_calloc(1,
-                                                sizeof(PagedResults));
+                conn->c_pagedresults.prl_list = (PagedResults *)slapi_ch_calloc(1, sizeof(PagedResults));
             } else {
                 /* new max length */
                 conn->c_pagedresults.prl_maxlen *= 2;
-                conn->c_pagedresults.prl_list =
-                    (PagedResults *)slapi_ch_realloc(
+                conn->c_pagedresults.prl_list = (PagedResults *)slapi_ch_realloc(
                                 (char *)conn->c_pagedresults.prl_list,
-                                sizeof(PagedResults) *
-                                conn->c_pagedresults.prl_maxlen);
+                                sizeof(PagedResults) * conn->c_pagedresults.prl_maxlen);
                 /* initialze newly allocated area */
-                memset(conn->c_pagedresults.prl_list + maxlen, '\0',
-                           sizeof(PagedResults) * maxlen);
+                memset(conn->c_pagedresults.prl_list + maxlen, '\0', sizeof(PagedResults) * maxlen);
             }
             *index = maxlen; /* the first position in the new area */
         } else {
@@ -276,8 +270,8 @@ pagedresults_free_one( Connection *conn, Operation *op, int index )
                 prp->pr_current_be->be_search_results_release &&
                 prp->pr_search_result_set) {
                 prp->pr_current_be->be_search_results_release(&(prp->pr_search_result_set));
-                prp->pr_current_be = NULL;
             }
+            prp->pr_current_be = NULL;
             if (prp->pr_mutex) {
                 /* pr_mutex is reused; back it up and reset it. */
                 prmutex = prp->pr_mutex;
@@ -314,8 +308,8 @@ pagedresults_free_one_msgid_nolock( Connection *conn, ber_int_t msgid )
                         prp->pr_current_be->be_search_results_release &&
                         prp->pr_search_result_set) {
                         prp->pr_current_be->be_search_results_release(&(prp->pr_search_result_set));
-                        prp->pr_current_be = NULL;
                     }
+                    prp->pr_current_be = NULL;
                     prp->pr_flags |= CONN_FLAG_PAGEDRESULTS_ABANDONED;
                     prp->pr_flags &= ~CONN_FLAG_PAGEDRESULTS_PROCESSING;
                     conn->c_pagedresults.prl_count--;
@@ -404,16 +398,8 @@ pagedresults_set_search_result(Connection *conn, Operation *op, void *sr,
     if (conn && (index > -1)) {
         if (!locked) PR_Lock(conn->c_mutex);
         if (index < conn->c_pagedresults.prl_maxlen) {
-            if (sr) { /* set */
-                if (NULL ==
-                    conn->c_pagedresults.prl_list[index].pr_search_result_set) {
-                    conn->c_pagedresults.prl_list[index].pr_search_result_set = sr;
-                    rc = 0;
-                }
-            } else {  /* reset */
-                conn->c_pagedresults.prl_list[index].pr_search_result_set = sr;
-                rc = 0;
-            }
+            conn->c_pagedresults.prl_list[index].pr_search_result_set = sr;
+            rc = 0;
         }
         if (!locked) PR_Unlock(conn->c_mutex);
     }
@@ -732,9 +718,9 @@ pagedresults_cleanup(Connection *conn, int needlock)
         if (prp->pr_current_be && prp->pr_search_result_set &&
             prp->pr_current_be->be_search_results_release) {
             prp->pr_current_be->be_search_results_release(&(prp->pr_search_result_set));
-            prp->pr_current_be = NULL;
             rc = 1;
         }
+        prp->pr_current_be = NULL;
         if (prp->pr_mutex) {
             PR_DestroyLock(prp->pr_mutex);
         }
@@ -780,9 +766,9 @@ pagedresults_cleanup_all(Connection *conn, int needlock)
         if (prp->pr_current_be && prp->pr_search_result_set &&
             prp->pr_current_be->be_search_results_release) {
             prp->pr_current_be->be_search_results_release(&(prp->pr_search_result_set));
-            prp->pr_current_be = NULL;
             rc = 1;
         }
+        prp->pr_current_be = NULL;
     }
     slapi_ch_free((void **)&conn->c_pagedresults.prl_list);
     conn->c_pagedresults.prl_maxlen = 0;
-- 
1.9.3