From 627d47e423e6c1c113f24ef7f29a2e5728cf70e4 Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Sun, 26 Apr 2015 14:46:44 -0700 Subject: [PATCH 316/319] 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. https://fedorahosted.org/389/ticket/48146 Reviewed by mreynolds@redhat.com (Thank you, Mark!!) (cherry picked from commit 947477f2e2367337a8c220d8c9d03a62bf1bbf1c) (cherry picked from commit ec8801a69adbe2f502513fabcbb79bc5e38bf197) (cherry picked from commit c4ab1196a597ff6845535cf5068be7c5524ec0cb) (cherry picked from commit 31cd2a52cc6a8b73a26016a0e1e6910c265a44d7) --- 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 bef19d1..3e55729 100644 --- a/ldap/servers/slapd/opshared.c +++ b/ldap/servers/slapd/opshared.c @@ -874,7 +874,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 */ @@ -890,17 +893,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