Patches for https://bugzilla.samba.org/show_bug.cgi?id=11182

[Michael Adam]

On 2015-05-06 at 15:59 -0700, Jeremy Allison wrote:
> On Wed, May 06, 2015 at 09:29:00PM +0200, Michael Adam wrote:
> > 
> > Metze's argument (I briefly discussed with him) is that
> > it was this way before. And since only the last in a
> > compound (except for oplock breaking create) can go async
> > and is taken out of the compound then, we are not in danger.
> 
> OK, thought through the logic really carefully.
> 
> If we have a pending notify on the processing queue
> not in a compound, then when smbd_smb2_session_setup_wrap_setup_done()
> gets called, it calls smb2srv_session_shutdown_send()
> which will call tevent_req_cancel() on any pending
> notifies.
> 
> Metze is (of course :-) correct in that we will
> *always* call tevent_req_cancel() on a notify,
> as the cannot be inside a compound, and can only
> be at the end - in which case it's split out.

I probably need to understand better how we process
these notifies in a compound, i.e. when the get
split out, but the concern was what happens
when the session_shutdown is called when we are still
occupied with the earlier parts of the compound and
have not yet split the notify out. If that can not
happen - great! (I will revisit the relevant code in
order to fully understand..)

> > But the concern is this: doesn't the introduction of the
> > Queue introduce a new potential for blocking (by prevent
> > the session from being freed waiting for the notify to disappear)?
> 
> No, I don't think so (and remember the tevent_queue was
> already there, metze's patch just moved it).

Gosh right, I was mislead.

Anyways, attached find the add-on patch that factors duplicate
code out into a function.

Cheers - Michael
-------------- next part --------------
From 5294246b6f9bde847d3877c5b23999c606466f30 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 6 May 2015 17:20:55 +0200
Subject: [PATCH] s3:smbXsrv: refactor duplicate code into
 smbXsrv_session_clear_and_logoff()

This replaces code in smbXsrv_session_logoff_all_callback()
and smbXsrv_session_clear_and_logoff().

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/smbXsrv_session.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index 2ccae0e..07375d8 100644
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -1094,7 +1094,7 @@ NTSTATUS smb2srv_session_close_previous_recv(struct tevent_req *req)
 	return NT_STATUS_OK;
 }
 
-static int smbXsrv_session_destructor(struct smbXsrv_session *session)
+static NTSTATUS smbXsrv_session_clear_and_logoff(struct smbXsrv_session *session)
 {
 	NTSTATUS status;
 	struct smbXsrv_connection *xconn = NULL;
@@ -1122,6 +1122,14 @@ static int smbXsrv_session_destructor(struct smbXsrv_session *session)
 	}
 
 	status = smbXsrv_session_logoff(session);
+	return status;
+}
+
+static int smbXsrv_session_destructor(struct smbXsrv_session *session)
+{
+	NTSTATUS status;
+
+	status = smbXsrv_session_clear_and_logoff(session);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(0, ("smbXsrv_session_destructor: "
 			  "smbXsrv_session_logoff() failed: %s\n",
@@ -1655,7 +1663,6 @@ static int smbXsrv_session_logoff_all_callback(struct db_record *local_rec,
 	TDB_DATA val;
 	void *ptr = NULL;
 	struct smbXsrv_session *session = NULL;
-	struct smbXsrv_connection *xconn = NULL;
 	NTSTATUS status;
 
 	val = dbwrap_record_get_value(local_rec);
@@ -1673,28 +1680,7 @@ static int smbXsrv_session_logoff_all_callback(struct db_record *local_rec,
 
 	session->db_rec = local_rec;
 
-	if (session->client != NULL) {
-		xconn = session->client->connections;
-	}
-	for (; xconn != NULL; xconn = xconn->next) {
-		struct smbd_smb2_request *preq;
-
-		for (preq = xconn->smb2.requests; preq != NULL; preq = preq->next) {
-			if (preq->session != session) {
-				continue;
-			}
-
-			preq->session = NULL;
-			/*
-			 * If we no longer have a session we can't
-			 * sign or encrypt replies.
-			 */
-			preq->do_signing = false;
-			preq->do_encryption = false;
-		}
-	}
-
-	status = smbXsrv_session_logoff(session);
+	status = smbXsrv_session_clear_and_logoff(session);
 	if (!NT_STATUS_IS_OK(status)) {
 		if (NT_STATUS_IS_OK(state->first_status)) {
 			state->first_status = status;
-- 
2.1.0

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150507/6763b935/attachment.pgp>