Browse Source

Trac Ticket #498 - Cannot abaondon simple paged result search

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

Bug Description: The enhancement "Ticket #260 - 389 DS does not
support multiple paging controls on a single connection (commit
add880accaa28de8304da1c2c2f58fe8af002ebb)" broke the ability to
abandon the on-going simple paged result search.

1) The abandon request expects the operation exist. When sending
an abort request, the search operation could have already finished
and the operation object has been released.

2) Plus, request page size is 0, it should be interpreted as abandoned.

Fix Description:
1) In do_abandon, this patch eliminates to check if the operation
is a simplepaged results oriented or not, since the operation object
is often already released.  Instead, it directly checks the internal
paged results info in the connection object.

To make sure the abandoned search won't go further, a flag value
CONN_FLAG_PAGEDRESULTS_ABANDONED is introduced.  If it is set in
the pagedresults structure in the connection object, it skips any
further process of the search.

2) This patch is adding a check if the given page size is 0 in the
simple-paged-results control or not.  If it is 0, treat is as an
abandoned operation.
Noriko Hosoi 13 years ago
parent
commit
47c0d96ac2

+ 4 - 6
ldap/servers/slapd/abandon.c

@@ -152,12 +152,10 @@ do_abandon( Slapi_PBlock *pb )
 		    0 );
 		    0 );
 	}
 	}
 
 
-	if ( op_is_pagedresults(o) ) {
-		if ( 0 == pagedresults_free_one_msgid_nolock(pb->pb_conn, id) ) {
-			slapi_log_access( LDAP_DEBUG_STATS, "conn=%" NSPRIu64 
-			                  " op=%d ABANDON targetop=Simple Paged Results\n",
-			                  pb->pb_conn->c_connid, pb->pb_op->o_opid );
-		}
+	if ( 0 == pagedresults_free_one_msgid_nolock(pb->pb_conn, id) ) {
+		slapi_log_access( LDAP_DEBUG_STATS, "conn=%" NSPRIu64 
+		                  " op=%d ABANDON targetop=Simple Paged Results\n",
+		                  pb->pb_conn->c_connid, pb->pb_op->o_opid );
 	} else if ( NULL == o ) {
 	} else if ( NULL == o ) {
 		slapi_log_access( LDAP_DEBUG_STATS, "conn=%" NSPRIu64 " op=%d ABANDON"
 		slapi_log_access( LDAP_DEBUG_STATS, "conn=%" NSPRIu64 " op=%d ABANDON"
 			" targetop=NOTFOUND msgid=%d\n",
 			" targetop=NOTFOUND msgid=%d\n",

+ 15 - 4
ldap/servers/slapd/opshared.c

@@ -189,7 +189,7 @@ modify_update_last_modified_attr(Slapi_PBlock *pb, Slapi_Mods *smods)
             /* anonymous bind */
             /* anonymous bind */
             bv.bv_val = "";
             bv.bv_val = "";
             bv.bv_len = 0;
             bv.bv_len = 0;
-   	    } else {
+        } else {
             bv.bv_val = binddn;
             bv.bv_val = binddn;
             bv.bv_len = strlen(bv.bv_val);
             bv.bv_len = strlen(bv.bv_val);
         }
         }
@@ -204,8 +204,8 @@ modify_update_last_modified_attr(Slapi_PBlock *pb, Slapi_Mods *smods)
         }
         }
     }
     }
 
 
-   	slapi_mods_add_modbvps(smods, LDAP_MOD_REPLACE | LDAP_MOD_BVALUES,
-   						   "modifiersname", bvals);
+    slapi_mods_add_modbvps(smods, LDAP_MOD_REPLACE | LDAP_MOD_BVALUES,
+                           "modifiersname", bvals);
 
 
     /* fill in modifytimestamp */
     /* fill in modifytimestamp */
     curtime = current_time();
     curtime = current_time();
@@ -468,7 +468,8 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
                                                 &pagesize, &pr_idx);
                                                 &pagesize, &pr_idx);
           /* Let's set pr_idx even if it fails; in case, pr_idx == -1. */
           /* Let's set pr_idx even if it fails; in case, pr_idx == -1. */
           slapi_pblock_set(pb, SLAPI_PAGED_RESULTS_INDEX, &pr_idx);
           slapi_pblock_set(pb, SLAPI_PAGED_RESULTS_INDEX, &pr_idx);
-          if (LDAP_SUCCESS == rc) {
+          if ((LDAP_SUCCESS == rc) ||
+              (LDAP_CANCELLED == rc) || (0 == pagesize)) {
               unsigned int opnote = SLAPI_OP_NOTE_SIMPLEPAGED;
               unsigned int opnote = SLAPI_OP_NOTE_SIMPLEPAGED;
               if (pagedresults_check_or_set_processing(pb->pb_conn, pr_idx)) {
               if (pagedresults_check_or_set_processing(pb->pb_conn, pr_idx)) {
                   send_ldap_result(pb, LDAP_UNWILLING_TO_PERFORM,
                   send_ldap_result(pb, LDAP_UNWILLING_TO_PERFORM,
@@ -490,6 +491,16 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
                   opnote |= SLAPI_OP_NOTE_UNINDEXED;
                   opnote |= SLAPI_OP_NOTE_UNINDEXED;
               }
               }
               slapi_pblock_set( pb, SLAPI_OPERATION_NOTES, &opnote );
               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);
+                  send_ldap_result(pb, 0, NULL,
+                                   "Simple Paged Results Search abandoned",
+                                   0, NULL);
+                  rc = LDAP_SUCCESS;
+                  goto free_and_return;
+              }
           } else {
           } else {
               /* parse paged-results-control failed */
               /* parse paged-results-control failed */
               if (iscritical) { /* return an error since it's critical */
               if (iscritical) { /* return an error since it's critical */

+ 28 - 13
ldap/servers/slapd/pagedresults.c

@@ -143,8 +143,13 @@ pagedresults_parse_control_value( Slapi_PBlock *pb,
     slapi_ch_free((void **)&cookie.bv_val);
     slapi_ch_free((void **)&cookie.bv_val);
 
 
     if ((*index > -1) && (*index < conn->c_pagedresults.prl_maxlen)) {
     if ((*index > -1) && (*index < conn->c_pagedresults.prl_maxlen)) {
-        /* Need to keep the latest msgid to prepare for the abandon. */
-        conn->c_pagedresults.prl_list[*index].pr_msgid = op->o_msgid;
+        if (conn->c_pagedresults.prl_list[*index].pr_flags &
+            CONN_FLAG_PAGEDRESULTS_ABANDONED) {
+            rc = LDAP_CANCELLED;
+        } else {
+            /* Need to keep the latest msgid to prepare for the abandon. */
+            conn->c_pagedresults.prl_list[*index].pr_msgid = op->o_msgid;
+        }
     } else {
     } else {
         rc = LDAP_PROTOCOL_ERROR;
         rc = LDAP_PROTOCOL_ERROR;
         LDAPDebug1Arg(LDAP_DEBUG_ANY,
         LDAPDebug1Arg(LDAP_DEBUG_ANY,
@@ -251,8 +256,13 @@ pagedresults_free_one( Connection *conn, int index )
                            "conn=%d paged requests list count is %d\n",
                            "conn=%d paged requests list count is %d\n",
                            conn->c_connid, conn->c_pagedresults.prl_count);
                            conn->c_connid, conn->c_pagedresults.prl_count);
         } else if (index < conn->c_pagedresults.prl_maxlen) {
         } else if (index < conn->c_pagedresults.prl_maxlen) {
-            memset(&conn->c_pagedresults.prl_list[index],
-                   '\0', sizeof(PagedResults));
+            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));
+            }
+            memset(prp, '\0', sizeof(PagedResults));
             conn->c_pagedresults.prl_count--;
             conn->c_pagedresults.prl_count--;
             rc = 0;
             rc = 0;
         }
         }
@@ -263,34 +273,39 @@ pagedresults_free_one( Connection *conn, int index )
     return rc;
     return rc;
 }
 }
 
 
+/* Used for abandoning */
 int 
 int 
 pagedresults_free_one_msgid_nolock( Connection *conn, ber_int_t msgid )
 pagedresults_free_one_msgid_nolock( Connection *conn, ber_int_t msgid )
 {
 {
     int rc = -1;
     int rc = -1;
     int i;
     int i;
 
 
-    LDAPDebug1Arg(LDAP_DEBUG_TRACE,
-                  "--> pagedresults_free_one: msgid=%d\n", msgid);
     if (conn && (msgid > -1)) {
     if (conn && (msgid > -1)) {
         if (conn->c_pagedresults.prl_count <= 0) {
         if (conn->c_pagedresults.prl_count <= 0) {
-            LDAPDebug2Args(LDAP_DEBUG_TRACE,
-                           "pagedresults_free_one_msgid_nolock: "
-                           "conn=%d paged requests list count is %d\n",
-                           conn->c_connid, conn->c_pagedresults.prl_count);
+            ; /* Not a paged result. */
         } else {
         } else {
+            LDAPDebug1Arg(LDAP_DEBUG_TRACE,
+                          "--> pagedresults_free_one: msgid=%d\n", msgid);
             for (i = 0; i < conn->c_pagedresults.prl_maxlen; i++) {
             for (i = 0; i < conn->c_pagedresults.prl_maxlen; i++) {
                 if (conn->c_pagedresults.prl_list[i].pr_msgid == msgid) {
                 if (conn->c_pagedresults.prl_list[i].pr_msgid == msgid) {
-                    memset(&conn->c_pagedresults.prl_list[i],
-                           '\0', sizeof(PagedResults));
+                    PagedResults *prp = conn->c_pagedresults.prl_list + i;
+                    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_flags |= CONN_FLAG_PAGEDRESULTS_ABANDONED;
+                    prp->pr_flags &= ~CONN_FLAG_PAGEDRESULTS_PROCESSING;
                     conn->c_pagedresults.prl_count--;
                     conn->c_pagedresults.prl_count--;
                     rc = 0;
                     rc = 0;
                     break;
                     break;
                 }
                 }
             }
             }
+            LDAPDebug1Arg(LDAP_DEBUG_TRACE,
+                          "<-- pagedresults_free_one: %d\n", rc);
         }
         }
     }
     }
 
 
-    LDAPDebug1Arg(LDAP_DEBUG_TRACE, "<-- pagedresults_free_one: %d\n", rc);
     return rc;
     return rc;
 }
 }
 
 

+ 8 - 7
ldap/servers/slapd/slap.h

@@ -1470,16 +1470,17 @@ typedef struct conn {
                                      * successfully completed.
                                      * successfully completed.
                                      */
                                      */
 
 
-#define CONN_FLAG_PAGEDRESULTS_WITH_SORT 64 /* paged results control is
-                                             * sent with server side sorting
-                                             */
+#define CONN_FLAG_PAGEDRESULTS_WITH_SORT   64/* paged results control is
+                                              * sent with server side sorting
+                                              */
 
 
-#define CONN_FLAG_PAGEDRESULTS_UNINDEXED 128 /* If the search is unindexed,
+#define CONN_FLAG_PAGEDRESULTS_UNINDEXED  128/* If the search is unindexed,
                                               * store the info in c_flags
                                               * store the info in c_flags
                                               */
                                               */
-#define CONN_FLAG_PAGEDRESULTS_PROCESSING 256 /* there is an operation
-					       * processing a pagedresults search
-					       */
+#define CONN_FLAG_PAGEDRESULTS_PROCESSING 256/* there is an operation
+                                              * processing a pagedresults search
+                                              */
+#define CONN_FLAG_PAGEDRESULTS_ABANDONED  512/* pagedresults abandoned */
 #define CONN_GET_SORT_RESULT_CODE (-1)
 #define CONN_GET_SORT_RESULT_CODE (-1)
 
 
 #define START_TLS_OID    "1.3.6.1.4.1.1466.20037"
 #define START_TLS_OID    "1.3.6.1.4.1.1466.20037"