From 22b0f8f0de1ee208a8c06c171bc2069da0adcc87 Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Mon, 6 Jul 2015 14:06:11 -0700 Subject: [PATCH] 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 rmeggins@redhat.com (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 rmeggins@redhat.com (Thank you, Rich!!) (cherry picked from commit e4d83c91fc88fcf9e6c823c608c629ac10e362f8) (cherry picked from commit b513a502250f93cfb43df000c2140b27c4ef0d39) (cherry picked from commit 7b17c488de280f29264920b4e53dce862ed5b7e4) --- ldap/servers/slapd/opshared.c | 22 ++++++++++++++++------ ldap/servers/slapd/pagedresults.c | 24 +++++++++++++++++++++++- ldap/servers/slapd/proto-slap.h | 1 + 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c index 4be5366..1bad7ef 100644 --- a/ldap/servers/slapd/opshared.c +++ b/ldap/servers/slapd/opshared.c @@ -709,12 +709,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; } @@ -744,7 +752,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 */ diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c index a7fe2cd..010e5c1 100644 --- a/ldap/servers/slapd/pagedresults.c +++ b/ldap/servers/slapd/pagedresults.c @@ -890,6 +890,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) @@ -918,7 +920,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) { @@ -981,7 +986,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); } @@ -995,9 +1002,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; +} diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h index c5c412d..4c0d1ce 100644 --- a/ldap/servers/slapd/proto-slap.h +++ b/ldap/servers/slapd/proto-slap.h @@ -1515,6 +1515,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 -- 1.9.3