Преглед изворни кода

Ticket #48192 - Individual abandoned simple paged results request has no chance to be cleaned up

Description: There was a small window that the search on the next page
after the previous page abandoned referred the cleaned up simple paged
object.

This patch introduces a pagedresults_is_abandoned helper function to
check the simple paged results was abandoned or not with some improvements
based upon the comments by [email protected] (Thank you!!):
1) adding locking when getting a simplepaged object in pagedresults_is_
   abandoned_or_notavailable as well as in pagedresults_{un}lock.
2) sending "Simple Paged Results Search abandoned" if the previous page
   with the same cookie in the same connection was abandoned.

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

Reviewed by [email protected] (Thank you, Rich!!)
Noriko Hosoi пре 10 година
родитељ
комит
e4d83c91fc
3 измењених фајлова са 40 додато и 7 уклоњено
  1. 16 6
      ldap/servers/slapd/opshared.c
  2. 23 1
      ldap/servers/slapd/pagedresults.c
  3. 1 0
      ldap/servers/slapd/proto-slap.h

+ 16 - 6
ldap/servers/slapd/opshared.c

@@ -677,12 +677,20 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
        */
       pr_search_result = pagedresults_get_search_result(pb->pb_conn, operation, pr_idx);
       if (pr_search_result) {
-        slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, pr_search_result );
-        rc = send_results_ext (pb, 1, &pnentries, pagesize, &pr_stat);
+        if (pagedresults_is_abandoned_or_notavailable(pb->pb_conn, 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;
+          goto free_and_return;
+        } else {
+          slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_SET, pr_search_result );
+          rc = send_results_ext (pb, 1, &pnentries, pagesize, &pr_stat);
 
-        /* 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);
+          /* 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);
+        }
       } else {
         pr_stat = PAGEDRESULTS_SEARCH_END;
       }
@@ -712,7 +720,9 @@ 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);
-        pagedresults_free_one(pb->pb_conn, operation, pr_idx);
+        if (!pagedresults_is_abandoned_or_notavailable(pb->pb_conn, pr_idx)) {
+          pagedresults_free_one(pb->pb_conn, operation, pr_idx);
+        }
         pagedresults_unlock(pb->pb_conn, pr_idx);
         if (next_be) {
           /* no more entries, but at least another backend */

+ 23 - 1
ldap/servers/slapd/pagedresults.c

@@ -877,6 +877,8 @@ pagedresults_reset_processing(Connection *conn, int index)
  * 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).
+ *
+ * must be called within conn->c_mutex 
  */
 int
 pagedresults_is_timedout_nolock(Connection *conn)
@@ -905,7 +907,10 @@ pagedresults_is_timedout_nolock(Connection *conn)
     return 0;
 }
 
-/* reset all timeout */
+/* 
+ * reset all timeout
+ * must be called within conn->c_mutex 
+ */
 int
 pagedresults_reset_timedout_nolock(Connection *conn)
 {
@@ -968,7 +973,9 @@ pagedresults_lock( Connection *conn, int index )
     if (!conn || (index < 0) || (index >= conn->c_pagedresults.prl_maxlen)) {
         return;
     }
+    PR_Lock(conn->c_mutex);
     prp = conn->c_pagedresults.prl_list + index;
+    PR_Unlock(conn->c_mutex);
     if (prp->pr_mutex) {
         PR_Lock(prp->pr_mutex);
     }
@@ -982,9 +989,24 @@ pagedresults_unlock( Connection *conn, int index )
     if (!conn || (index < 0) || (index >= conn->c_pagedresults.prl_maxlen)) {
         return;
     }
+    PR_Lock(conn->c_mutex);
     prp = conn->c_pagedresults.prl_list + index;
+    PR_Unlock(conn->c_mutex);
     if (prp->pr_mutex) {
         PR_Unlock(prp->pr_mutex);
     }
     return;
 }
+
+int
+pagedresults_is_abandoned_or_notavailable( Connection *conn, 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);
+    prp = conn->c_pagedresults.prl_list + index;
+    PR_Unlock(conn->c_mutex);
+    return prp->pr_flags & CONN_FLAG_PAGEDRESULTS_ABANDONED;
+}

+ 1 - 0
ldap/servers/slapd/proto-slap.h

@@ -1487,6 +1487,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);
 
 /*
  * sort.c