Skip to content

Commit

Permalink
Ticket #48192 - Individual abandoned simple paged results request has…
Browse files Browse the repository at this point in the history
… 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 e4d83c9)
(cherry picked from commit b513a50)
  • Loading branch information
nhosoi committed Jul 7, 2015
1 parent a71b48c commit 7a05195
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 7 deletions.
22 changes: 16 additions & 6 deletions ldap/servers/slapd/opshared.c
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 */
Expand Down
24 changes: 23 additions & 1 deletion ldap/servers/slapd/pagedresults.c
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
}
1 change: 1 addition & 0 deletions ldap/servers/slapd/proto-slap.h
Expand Up @@ -1524,6 +1524,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
Expand Down

0 comments on commit 7a05195

Please sign in to comment.