[SCM] Samba Shared Repository - branch master updated

Noel Power npower at samba.org
Wed Aug 3 14:01:01 UTC 2022


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 <slow at 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 <slow at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>
    
    Autobuild-User(master): Noel Power <npower at samba.org>
    Autobuild-Date(master): Wed Aug  3 14:00:36 UTC 2022 on sn-devel-184

commit c9ecd33ad7db1ebf0b45c84b3909da7f5d719856
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>

commit ac13935a58518a3af34fd49701846b8dbe72b7b0
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>

commit 1150d121b7f6588de1aa37eac810c19dbfc07a71
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>

commit 9b0e61ff75db0d875da81ada6d2333b01985d264
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>

commit 9b56c7030f86f24a5b21f2a972a641afb556f7ab
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>

commit 2fc2c7d4b0b9e5351a6f4f4e3c574e8504b0a536
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>

commit 5b750d6b330a53f96924106eddb5be4224a5fc4a
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>

commit c0d46796d435174ff71ede9175097fc01546d69f
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>

commit 3254622a307dde7ca12d90ceb58336a6948fa6d2
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Noel Power <npower at 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



More information about the samba-cvs mailing list