Blob Blame Raw
From 810e825adebf9f1669bab9f61831f158e0c2f941 Mon Sep 17 00:00:00 2001
From: Noriko Hosoi <nhosoi@redhat.com>
Date: Thu, 4 Jun 2015 16:27:05 -0700
Subject: [PATCH 70/72] Ticket #48192 - Individual abandoned simple paged
 results request has no chance to be cleaned up

Description: When allocating a slot in simple paged results array stashed
in a connection in pagedresults_parse_control_value, a new code is added
to scan the array if the existing slot is timed out or not.  If it is, the
slot is released and a search results is released if it is attached.

Also, if a request is abandoned, instead of returning a valid cookie, it
changed to return an empty cookie to inform the client the request was not
successfully completed.

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

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

(cherry picked from commit f3c3125fc9fd4dc1cbd41027d6442cb525efd014)
(cherry picked from commit 97c707df4e11936202310e99bcf5f45d15b49f0f)
---
 ldap/servers/slapd/opshared.c     |  41 +++++++-------
 ldap/servers/slapd/pagedresults.c | 110 ++++++++++++++++++++++++--------------
 2 files changed, 89 insertions(+), 62 deletions(-)

diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c
index 5dff42f..9cf431c 100644
--- a/ldap/servers/slapd/opshared.c
+++ b/ldap/servers/slapd/opshared.c
@@ -517,8 +517,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
           rc = pagedresults_parse_control_value(pb, ctl_value, &pagesize, &pr_idx, be);
           /* Let's set pr_idx even if it fails; in case, pr_idx == -1. */
           slapi_pblock_set(pb, SLAPI_PAGED_RESULTS_INDEX, &pr_idx);
-          if ((LDAP_SUCCESS == rc) ||
-              (LDAP_CANCELLED == rc) || (0 == pagesize)) {
+          if ((LDAP_SUCCESS == rc) || (LDAP_CANCELLED == rc) || (0 == pagesize)) {
               unsigned int opnote = SLAPI_OP_NOTE_SIMPLEPAGED;
               op_set_pagedresults(operation);
               pr_be = pagedresults_get_current_be(pb->pb_conn, pr_idx);
@@ -545,9 +544,8 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
               }
               slapi_pblock_set( pb, SLAPI_OPERATION_NOTES, &opnote );
               if ((LDAP_CANCELLED == rc) || (0 == pagesize)) {
-                  /* paged-results-request was abandoned */
-                  pagedresults_set_response_control(pb, 0, estimate, 
-                                                    curr_search_count, pr_idx);
+                  /* paged-results-request was abandoned; making an empty cookie. */
+                  pagedresults_set_response_control(pb, 0, estimate, -1, pr_idx);
                   send_ldap_result(pb, 0, NULL,
                                    "Simple Paged Results Search abandoned",
                                    0, NULL);
@@ -574,10 +572,21 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
 
       /* adjust time and size limits */
       compute_limits (pb);
-    
-      /* call the pre-search plugins. if they succeed, call the backend 
-     search function. then call the post-search plugins. */
 
+      /* set the timelimit to clean up the too-long-lived-paged results requests */
+      if (op_is_pagedresults(operation)) {
+        time_t optime, time_up;
+        int tlimit;
+        slapi_pblock_get( pb, SLAPI_SEARCH_TIMELIMIT, &tlimit );
+        slapi_pblock_get( pb, SLAPI_OPINITIATED_TIME, &optime );
+        time_up = (tlimit==-1 ? -1 : optime + tlimit); /* -1: no time limit */
+        pagedresults_set_timelimit(pb->pb_conn, pb->pb_op, time_up, pr_idx);
+      }
+    
+      /*
+       * call the pre-search plugins. if they succeed, call the backend 
+       * search function. then call the post-search plugins.
+       */
       /* ONREPL - should regular plugin be called for internal searches ? */
       if (plugin_call_plugins(pb, SLAPI_PLUGIN_PRE_SEARCH_FN) == 0)
       {
@@ -642,16 +651,6 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
     }
   }
 
-  /* set the timelimit to clean up the too-long-lived-paged results requests */
-  if (op_is_pagedresults(operation)) {
-    time_t optime, time_up;
-    int tlimit;
-    slapi_pblock_get( pb, SLAPI_SEARCH_TIMELIMIT, &tlimit );
-    slapi_pblock_get( pb, SLAPI_OPINITIATED_TIME, &optime );
-    time_up = (tlimit==-1 ? -1 : optime + tlimit); /* -1: no time limit */
-    pagedresults_set_timelimit(pb->pb_conn, pb->pb_op, time_up, pr_idx);
-  }
-
   /* PAR: now filters have been rewritten, we can assign plugins to work on them */
   index_subsys_assign_filter_decoders(pb);
 
@@ -880,10 +879,7 @@ 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);
+                pagedresults_free_one(pb->pb_conn, operation, pr_idx);
               }
               if (NULL == next_be) {
                 /* no more entries && no more backends */
@@ -1306,7 +1302,6 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result,
                 operation_out_of_disk_space();
             }
             pr_stat = PAGEDRESULTS_SEARCH_END;
-            pagedresults_set_timelimit(pb->pb_conn, pb->pb_op, 0, pr_idx);
             rval = -1;
             done = 1;
             continue;
diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c
index 327da54..402dd10 100644
--- a/ldap/servers/slapd/pagedresults.c
+++ b/ldap/servers/slapd/pagedresults.c
@@ -41,6 +41,27 @@
 
 #include "slap.h"
 
+/* helper function to clean up one prp slot */
+static void
+_pr_cleanup_one_slot(PagedResults *prp)
+{
+    PRLock *prmutex = NULL;
+    if (!prp) {
+        return;
+    }
+    if (prp->pr_current_be && prp->pr_current_be->be_search_results_release) {
+        /* sr is left; release it. */
+        prp->pr_current_be->be_search_results_release(&(prp->pr_search_result_set));
+    }
+    /* clean up the slot */
+    if (prp->pr_mutex) {
+        /* pr_mutex is reused; back it up and reset it. */
+        prmutex = prp->pr_mutex;
+    }
+    memset(prp, '\0', sizeof(PagedResults));
+    prp->pr_mutex = prmutex;
+}
+
 /*
  * Parse the value from an LDAPv3 "Simple Paged Results" control.  They look
  * like this:
@@ -65,6 +86,7 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
     Connection *conn = pb->pb_conn;
     Operation *op = pb->pb_op;
     BerElement *ber = NULL;
+    PagedResults *prp = NULL;
 
     LDAPDebug0Args(LDAP_DEBUG_TRACE, "--> pagedresults_parse_control_value\n");
     if ( NULL == conn || NULL == op || NULL == pagesize || NULL == index ) {
@@ -117,13 +139,31 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
             }
             *index = maxlen; /* the first position in the new area */
         } else {
-            for (i = 0; i < conn->c_pagedresults.prl_maxlen; i++) {
-                if (!conn->c_pagedresults.prl_list[i].pr_current_be) {
-                    conn->c_pagedresults.prl_list[i].pr_current_be = be;
+            time_t ctime = current_time();
+            prp = conn->c_pagedresults.prl_list;
+            for (i = 0; i < conn->c_pagedresults.prl_maxlen; i++, prp++) {
+                if (!prp->pr_current_be) { /* unused slot; take it */
+                    prp->pr_current_be = be;
+                    *index = i;
+                    break;
+                } else if (((prp->pr_timelimit > 0) && (ctime < prp->pr_timelimit)) || /* timelimit exceeded */
+                           (prp->pr_flags & CONN_FLAG_PAGEDRESULTS_ABANDONED) /* abandoned */) {
+                    _pr_cleanup_one_slot(prp);
+                    conn->c_pagedresults.prl_count--;
+                    prp->pr_current_be = be;
                     *index = i;
                     break;
                 }
             }
+            /* cleaning up the rest of the timedout if any */
+            for (++i; i < conn->c_pagedresults.prl_maxlen; i++, prp++) {
+                if (prp->pr_current_be &&
+                    (((prp->pr_timelimit > 0) && (ctime < prp->pr_timelimit)) || /* timelimit exceeded */
+                    (prp->pr_flags & CONN_FLAG_PAGEDRESULTS_ABANDONED)) /* abandoned */) {
+                    _pr_cleanup_one_slot(prp);
+                    conn->c_pagedresults.prl_count--;
+                }
+            }
         }
         if ((*index > -1) && (*index < conn->c_pagedresults.prl_maxlen) &&
             !conn->c_pagedresults.prl_list[*index].pr_mutex) {
@@ -131,7 +171,6 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
         }
         conn->c_pagedresults.prl_count++;
     } else {
-        PagedResults *prp = NULL;
         /* Repeated paged results request.
          * PagedResults is already allocated. */
         char *ptr = slapi_ch_malloc(cookie.bv_len + 1);
@@ -156,8 +195,10 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
     slapi_ch_free((void **)&cookie.bv_val);
 
     if ((*index > -1) && (*index < conn->c_pagedresults.prl_maxlen)) {
-        if (conn->c_pagedresults.prl_list[*index].pr_flags &
-            CONN_FLAG_PAGEDRESULTS_ABANDONED) {
+        if (conn->c_pagedresults.prl_list[*index].pr_flags & CONN_FLAG_PAGEDRESULTS_ABANDONED) {
+            /* repeated case? */
+            prp = conn->c_pagedresults.prl_list + *index;
+            _pr_cleanup_one_slot(prp);
             rc = LDAP_CANCELLED;
         } else {
             /* Need to keep the latest msgid to prepare for the abandon. */
@@ -273,20 +314,8 @@ pagedresults_free_one( Connection *conn, Operation *op, int index )
                            "conn=%d paged requests list count is %d\n",
                            conn->c_connid, conn->c_pagedresults.prl_count);
         } else if (index < conn->c_pagedresults.prl_maxlen) {
-            PRLock *prmutex = NULL;
             PagedResults *prp = conn->c_pagedresults.prl_list + index;
-            if (prp && prp->pr_current_be &&
-                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;
-            if (prp->pr_mutex) {
-                /* pr_mutex is reused; back it up and reset it. */
-                prmutex = prp->pr_mutex;
-            }
-            memset(prp, '\0', sizeof(PagedResults));
-            prp->pr_mutex = prmutex;
+            _pr_cleanup_one_slot(prp);
             conn->c_pagedresults.prl_count--;
             rc = 0;
         }
@@ -309,7 +338,7 @@ pagedresults_free_one_msgid_nolock( Connection *conn, ber_int_t msgid )
             ; /* Not a paged result. */
         } else {
             LDAPDebug1Arg(LDAP_DEBUG_TRACE,
-                          "--> pagedresults_free_one: msgid=%d\n", msgid);
+                          "--> pagedresults_free_one_msgid_nolock: msgid=%d\n", msgid);
             for (i = 0; i < conn->c_pagedresults.prl_maxlen; i++) {
                 if (conn->c_pagedresults.prl_list[i].pr_msgid == msgid) {
                     PagedResults *prp = conn->c_pagedresults.prl_list + i;
@@ -318,16 +347,14 @@ pagedresults_free_one_msgid_nolock( Connection *conn, ber_int_t msgid )
                         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_flags |= CONN_FLAG_PAGEDRESULTS_ABANDONED;
                     prp->pr_flags &= ~CONN_FLAG_PAGEDRESULTS_PROCESSING;
-                    conn->c_pagedresults.prl_count--;
                     rc = 0;
                     break;
                 }
             }
             LDAPDebug1Arg(LDAP_DEBUG_TRACE,
-                          "<-- pagedresults_free_one: %d\n", rc);
+                          "<-- pagedresults_free_one_msgid_nolock: %d\n", rc);
         }
     }
 
@@ -845,37 +872,42 @@ pagedresults_reset_processing(Connection *conn, int index)
 }
 #endif
 
-/* Are all the paged results requests timed out? */
+/*
+ * This timedout is mainly for an end user leaves a commandline untouched
+ * for a long time.  This should not affect a permanent connection which
+ * manages multiple simple paged results requests over the connection.
+ *
+ * [rule]
+ * If there is just one slot and it's timed out, we return it is timedout.
+ * If there are multiple slots, the connection may be a permanent one.
+ * Do not return timed out here.  But let the next request take care the
+ * timedout slot(s).
+ */
 int
 pagedresults_is_timedout_nolock(Connection *conn)
 {
-    int i;
     PagedResults *prp = NULL;
     time_t ctime;
-    int rc = 0;
 
     LDAPDebug0Args(LDAP_DEBUG_TRACE, "--> pagedresults_is_timedout\n");
 
-    if (NULL == conn || 0 == conn->c_pagedresults.prl_count) {
-        LDAPDebug0Args(LDAP_DEBUG_TRACE, "<-- pagedresults_is_timedout: -\n");
-        return rc;
+    if (!conn || (0 == conn->c_pagedresults.prl_maxlen)) {
+        LDAPDebug0Args(LDAP_DEBUG_TRACE, "<-- pagedresults_is_timedout: false\n");
+        return 0;
     }
 
     ctime = current_time();
-    for (i = 0; i < conn->c_pagedresults.prl_maxlen; i++) {
-        prp = conn->c_pagedresults.prl_list + i;
+    prp = conn->c_pagedresults.prl_list;
+    if (prp && (1 == conn->c_pagedresults.prl_maxlen)) {
         if (prp->pr_current_be && (prp->pr_timelimit > 0)) {
-            if (ctime < prp->pr_timelimit) {
-                LDAPDebug0Args(LDAP_DEBUG_TRACE,
-                               "<-- pagedresults_is_timedout: 0\n");
-                return 0; /* at least, one request is not timed out. */
-            } else {
-                rc = 1;   /* possibly timed out */
+            if (ctime > prp->pr_timelimit) {
+                LDAPDebug0Args(LDAP_DEBUG_TRACE, "<-- pagedresults_is_timedout: true\n");
+                return 1;
             }
         }
     }
-    LDAPDebug0Args(LDAP_DEBUG_TRACE, "<-- pagedresults_is_timedout: 1\n");
-    return rc; /* all requests are timed out. */
+    LDAPDebug0Args(LDAP_DEBUG_TRACE, "<-- pagedresults_is_timedout: false\n");
+    return 0;
 }
 
 /* reset all timeout */
-- 
1.9.3