The branch, master has been updated via 61c6a00f550 mdssvc: check if the user closed the query before trying to read the HTTP response from Elasticsearch via c9ecd33ad7d mdssvc: fold two if blocks into one via ac13935a585 mdssvc: don't trigger http reconnect if a search was cancelled via 1150d121b7f mdssvc: fix check if search connection state is gone via 9b0e61ff75d mdssvc: reapply default search destructor when marking a search non-pending via 9b56c7030f8 mdssvc: prevent a crash when pending search finishes after the client closed the search connection via 2fc2c7d4b0b mdssvc: move calling mds_es_search_set_pending() to mds_es_next_search_trigger() via 5b750d6b330 mdssvc: consolidate calls of mds_es_search_unset_pending() via c0d46796d43 mdssvc: update a comment via 3254622a307 mdssvc: fix a comment from 93b6db3328c s3: smbd: Convert smb_file_rename_information() to use filename_convert_dirfsp().
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 61c6a00f550a6ffc8fe704e15bc44134befc40c8 Author: Ralph Boehme <s...@samba.org> Date: Fri Nov 19 13:24:50 2021 +0100 mdssvc: check if the user closed the query before trying to read the HTTP response from Elasticsearch BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> Autobuild-User(master): Noel Power <npo...@samba.org> Autobuild-Date(master): Wed Aug 3 14:00:36 UTC 2022 on sn-devel-184 commit c9ecd33ad7db1ebf0b45c84b3909da7f5d719856 Author: Ralph Boehme <s...@samba.org> Date: Fri Nov 19 16:50:44 2021 +0100 mdssvc: fold two if blocks into one No change in behaviour. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> commit ac13935a58518a3af34fd49701846b8dbe72b7b0 Author: Ralph Boehme <s...@samba.org> Date: Fri Nov 19 13:21:31 2021 +0100 mdssvc: don't trigger http reconnect if a search was cancelled Calling tevent_req_error() triggers a HTTP reconnect in mds_es_search_done() as mds_es_search_recv() returns the error so we call mds_es_reconnect_on_error(). slq (which is s->slq) or s->mds_es_ctx will be NULL if the user closed a search or disconnected a share with an active mdssvc IPC pipe, no need to trigger a HTTP reconnect for those cases. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> commit 1150d121b7f6588de1aa37eac810c19dbfc07a71 Author: Ralph Boehme <s...@samba.org> Date: Fri Nov 19 13:11:20 2021 +0100 mdssvc: fix check if search connection state is gone This was dead code: before this patchset noone set s->mds_es_ctx->mds_ctx to NULL. A previous commit changed that so now the mds_es_ctx destructor sets s->mds_es_ctx to NULL if a search "s" was currently in-flight. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> commit 9b0e61ff75db0d875da81ada6d2333b01985d264 Author: Ralph Boehme <s...@samba.org> Date: Thu Nov 18 16:51:36 2021 +0100 mdssvc: reapply default search destructor when marking a search non-pending This is needed to ensure searches that are scheduled more then once to the Elasticsarch server (because the first run didn't return all results) get removed from the list of searches in case the user closes the query. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> commit 9b56c7030f86f24a5b21f2a972a641afb556f7ab Author: Ralph Boehme <s...@samba.org> Date: Fri Nov 19 13:29:54 2021 +0100 mdssvc: prevent a crash when pending search finishes after the client closed the search connection When a search is in-flight and currently being processed against the Elasticsearch server, we set s->pending. In the destructor of "s" we check "pending" and reject deallocation of the object. One instance where "s" is requested to be deallocated is when the client closes the top-level per-share search connection. This will implicitly close all searches associated with the mds_ctx from mds_ctx_destructor_cb(): while (mds_ctx->query_list != NULL) { /* * slq destructor removes element from list. * Don't use TALLOC_FREE()! */ talloc_free(mds_ctx->query_list); } So when this happens the Elasticsearch backend query object stays around, alongside with any active tevent_req request and a tevent_req timer set with tevent_req_set_endtime() in mds_es_search_send(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915 RN: mdssvc crashes when searches are pending and the client closes the mdssvc IPC pipe Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> commit 2fc2c7d4b0b9e5351a6f4f4e3c574e8504b0a536 Author: Ralph Boehme <s...@samba.org> Date: Fri Nov 19 12:56:37 2021 +0100 mdssvc: move calling mds_es_search_set_pending() to mds_es_next_search_trigger() This makes the calls to mds_es_search_set_pending() and mds_es_search_unset_pending() symmetric. No change in behaviour. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> commit 5b750d6b330a53f96924106eddb5be4224a5fc4a Author: Ralph Boehme <s...@samba.org> Date: Fri Nov 19 13:28:17 2021 +0100 mdssvc: consolidate calls of mds_es_search_unset_pending() Both codepaths were mds_es_search_unset_pending() is currently called end up going through the higher level callback mds_es_search_done(). Moving the call to mds_es_search_unset_pending() ensures we call it consistently and don't miss it in some error code path. Otherwise no change in behaviour. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> commit c0d46796d435174ff71ede9175097fc01546d69f Author: Ralph Boehme <s...@samba.org> Date: Fri Nov 19 08:27:34 2021 +0100 mdssvc: update a comment BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> commit 3254622a307dde7ca12d90ceb58336a6948fa6d2 Author: Ralph Boehme <s...@samba.org> Date: Thu Nov 18 16:51:21 2021 +0100 mdssvc: fix a comment BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> ----------------------------------------------------------------------- Summary of changes: source3/rpc_server/mdssvc/mdssvc_es.c | 58 ++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 15 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/rpc_server/mdssvc/mdssvc_es.c b/source3/rpc_server/mdssvc/mdssvc_es.c index 7eba87abf53..dafb42610fa 100644 --- a/source3/rpc_server/mdssvc/mdssvc_es.c +++ b/source3/rpc_server/mdssvc/mdssvc_es.c @@ -112,6 +112,28 @@ static struct tevent_req *mds_es_connect_send( static int mds_es_connect_recv(struct tevent_req *req); static void mds_es_connected(struct tevent_req *subreq); static bool mds_es_next_search_trigger(struct mds_es_ctx *mds_es_ctx); +static void mds_es_search_set_pending(struct sl_es_search *s); +static void mds_es_search_unset_pending(struct sl_es_search *s); + +static int mds_es_ctx_destructor(struct mds_es_ctx *mds_es_ctx) +{ + struct sl_es_search *s = mds_es_ctx->searches; + + /* + * The per tree-connect state mds_es_ctx (a child of mds_ctx) is about + * to go away and has already freed all waiting searches. If there's a + * search remaining that's when the search is already active. Reset the + * mds_es_ctx pointer, so we can detect this when the search completes. + */ + + if (s == NULL) { + return 0; + } + + s->mds_es_ctx = NULL; + + return 0; +} static bool mds_es_connect(struct mds_ctx *mds_ctx) { @@ -130,6 +152,7 @@ static bool mds_es_connect(struct mds_ctx *mds_ctx) }; mds_ctx->backend_private = mds_es_ctx; + talloc_set_destructor(mds_es_ctx, mds_es_ctx_destructor); subreq = mds_es_connect_send( mds_es_ctx, @@ -347,6 +370,9 @@ static void mds_es_reconnect_on_error(struct sl_es_search *s) static int search_destructor(struct sl_es_search *s) { + if (s->mds_es_ctx == NULL) { + return 0; + } DLIST_REMOVE(s->mds_es_ctx->searches, s); return 0; } @@ -426,6 +452,7 @@ static bool mds_es_next_search_trigger(struct mds_es_ctx *mds_es_ctx) return false; } tevent_req_set_callback(subreq, mds_es_search_done, s); + mds_es_search_set_pending(s); return true; } @@ -440,6 +467,16 @@ static void mds_es_search_done(struct tevent_req *subreq) DBG_DEBUG("Search done for search [%p]\n", s); + mds_es_search_unset_pending(s); + + if (mds_es_ctx == NULL) { + /* + * Search connection closed by the user while s was pending. + */ + TALLOC_FREE(s); + return; + } + DLIST_REMOVE(mds_es_ctx->searches, s); ret = mds_es_search_recv(subreq); @@ -451,9 +488,8 @@ static void mds_es_search_done(struct tevent_req *subreq) if (slq == NULL) { /* - * Closed by the user. This is the only place where we free "s" - * explicitly because the talloc parent slq is already gone. - * Everywhere else we rely on the destructor of slq to free s"." + * Closed by the user. Explicitly free "s" here because the + * talloc parent slq is already gone. */ TALLOC_FREE(s); goto trigger; @@ -528,7 +564,7 @@ static void mds_es_search_unset_pending(struct sl_es_search *s) } s->pending = false; - talloc_set_destructor(s, NULL); + talloc_set_destructor(s, search_destructor); } static struct tevent_req *mds_es_search_send(TALLOC_CTX *mem_ctx, @@ -628,7 +664,6 @@ static struct tevent_req *mds_es_search_send(TALLOC_CTX *mem_ctx, if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } - mds_es_search_set_pending(s); tevent_req_set_callback(subreq, mds_es_search_http_send_done, req); return req; } @@ -650,9 +685,8 @@ static void mds_es_search_http_send_done(struct tevent_req *subreq) return; } - if (state->s->mds_es_ctx->mds_ctx == NULL) { - mds_es_search_unset_pending(state->s); - tevent_req_error(req, ECANCELED); + if (state->s->mds_es_ctx == NULL || state->s->slq == NULL) { + tevent_req_done(req); return; } @@ -686,8 +720,6 @@ static void mds_es_search_http_read_done(struct tevent_req *subreq) DBG_DEBUG("Got response for search [%p]\n", s); - mds_es_search_unset_pending(s); - status = http_read_response_recv(subreq, state, &state->http_response); TALLOC_FREE(subreq); if (!NT_STATUS_IS_OK(status)) { @@ -696,14 +728,10 @@ static void mds_es_search_http_read_done(struct tevent_req *subreq) return; } - if (slq == NULL) { + if (slq == NULL || s->mds_es_ctx == NULL) { tevent_req_done(req); return; } - if (s->mds_es_ctx->mds_ctx == NULL) { - tevent_req_error(req, ECANCELED); - return; - } switch (state->http_response->response_code) { case 200: -- Samba Shared Repository