From e905412d419267d7bbb5d833b5c4cd5aff0cf0dd Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Sun, 26 Apr 2015 14:46:44 -0700 Subject: [PATCH] Ticket #48146 - async simple paged results issue Description: When the last page of the single paged results is returned, the search results structure is freed in the simple paged results code (in op_shared_search). The search results structure is stashed in the simple paged results object across the pages. The free and the clean up of the stashed address should have been atomic, but it was not. --- ldap/servers/slapd/opshared.c | 18 +++++++----------- ldap/servers/slapd/pagedresults.c | 36 +++++++++++------------------------- 2 files changed, 18 insertions(+), 36 deletions(-) diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c index 84a2bb4..272e550 100644 --- a/ldap/servers/slapd/opshared.c +++ b/ldap/servers/slapd/opshared.c @@ -881,7 +881,10 @@ 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); } if (NULL == next_be) { /* no more entries && no more backends */ @@ -897,17 +900,10 @@ op_shared_search (Slapi_PBlock *pb, int send_result) slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate); pagedresults_lock(pb->pb_conn, pr_idx); if ((pagedresults_set_current_be(pb->pb_conn, be, pr_idx) < 0) || - (pagedresults_set_search_result(pb->pb_conn, operation, - sr, 0, pr_idx) < 0) || - (pagedresults_set_search_result_count(pb->pb_conn, operation, - curr_search_count, - pr_idx) < 0) || - (pagedresults_set_search_result_set_size_estimate(pb->pb_conn, - operation, - estimate, - pr_idx) < 0) || - (pagedresults_set_with_sort(pb->pb_conn, operation, - with_sort, pr_idx) < 0)) { + (pagedresults_set_search_result(pb->pb_conn, operation, sr, 0, pr_idx) < 0) || + (pagedresults_set_search_result_count(pb->pb_conn, operation, curr_search_count, pr_idx) < 0) || + (pagedresults_set_search_result_set_size_estimate(pb->pb_conn, operation, estimate, pr_idx) < 0) || + (pagedresults_set_with_sort(pb->pb_conn, operation, with_sort, pr_idx) < 0)) { pagedresults_unlock(pb->pb_conn, pr_idx); goto free_and_return; } diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c index d589708..e61c000 100644 --- a/ldap/servers/slapd/pagedresults.c +++ b/ldap/servers/slapd/pagedresults.c @@ -100,26 +100,20 @@ pagedresults_parse_control_value( Slapi_PBlock *pb, ber_free(ber, 1); if ( cookie.bv_len <= 0 ) { int i; - int maxlen; /* first time? */ - maxlen = conn->c_pagedresults.prl_maxlen; + int maxlen = conn->c_pagedresults.prl_maxlen; if (conn->c_pagedresults.prl_count == maxlen) { if (0 == maxlen) { /* first time */ conn->c_pagedresults.prl_maxlen = 1; - conn->c_pagedresults.prl_list = - (PagedResults *)slapi_ch_calloc(1, - sizeof(PagedResults)); + conn->c_pagedresults.prl_list = (PagedResults *)slapi_ch_calloc(1, sizeof(PagedResults)); } else { /* new max length */ conn->c_pagedresults.prl_maxlen *= 2; - conn->c_pagedresults.prl_list = - (PagedResults *)slapi_ch_realloc( + conn->c_pagedresults.prl_list = (PagedResults *)slapi_ch_realloc( (char *)conn->c_pagedresults.prl_list, - sizeof(PagedResults) * - conn->c_pagedresults.prl_maxlen); + sizeof(PagedResults) * conn->c_pagedresults.prl_maxlen); /* initialze newly allocated area */ - memset(conn->c_pagedresults.prl_list + maxlen, '\0', - sizeof(PagedResults) * maxlen); + memset(conn->c_pagedresults.prl_list + maxlen, '\0', sizeof(PagedResults) * maxlen); } *index = maxlen; /* the first position in the new area */ } else { @@ -276,8 +270,8 @@ pagedresults_free_one( Connection *conn, Operation *op, int index ) 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; } + prp->pr_current_be = NULL; if (prp->pr_mutex) { /* pr_mutex is reused; back it up and reset it. */ prmutex = prp->pr_mutex; @@ -314,8 +308,8 @@ pagedresults_free_one_msgid_nolock( Connection *conn, ber_int_t msgid ) 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; } + prp->pr_current_be = NULL; prp->pr_flags |= CONN_FLAG_PAGEDRESULTS_ABANDONED; prp->pr_flags &= ~CONN_FLAG_PAGEDRESULTS_PROCESSING; conn->c_pagedresults.prl_count--; @@ -404,16 +398,8 @@ pagedresults_set_search_result(Connection *conn, Operation *op, void *sr, if (conn && (index > -1)) { if (!locked) PR_Lock(conn->c_mutex); if (index < conn->c_pagedresults.prl_maxlen) { - if (sr) { /* set */ - if (NULL == - conn->c_pagedresults.prl_list[index].pr_search_result_set) { - conn->c_pagedresults.prl_list[index].pr_search_result_set = sr; - rc = 0; - } - } else { /* reset */ - conn->c_pagedresults.prl_list[index].pr_search_result_set = sr; - rc = 0; - } + conn->c_pagedresults.prl_list[index].pr_search_result_set = sr; + rc = 0; } if (!locked) PR_Unlock(conn->c_mutex); } @@ -732,9 +718,9 @@ pagedresults_cleanup(Connection *conn, int needlock) if (prp->pr_current_be && prp->pr_search_result_set && prp->pr_current_be->be_search_results_release) { prp->pr_current_be->be_search_results_release(&(prp->pr_search_result_set)); - prp->pr_current_be = NULL; rc = 1; } + prp->pr_current_be = NULL; if (prp->pr_mutex) { PR_DestroyLock(prp->pr_mutex); } @@ -780,9 +766,9 @@ pagedresults_cleanup_all(Connection *conn, int needlock) if (prp->pr_current_be && prp->pr_search_result_set && prp->pr_current_be->be_search_results_release) { prp->pr_current_be->be_search_results_release(&(prp->pr_search_result_set)); - prp->pr_current_be = NULL; rc = 1; } + prp->pr_current_be = NULL; } slapi_ch_free((void **)&conn->c_pagedresults.prl_list); conn->c_pagedresults.prl_maxlen = 0; -- 1.9.3