On Wed, 2013-06-26 at 20:39 +1000, Andrew Bartlett wrote:
> On Mon, 2013-06-24 at 15:26 +0000, philippe.simo...@swisscom.com wrote:
> > Hi Andrew, and by putting more num-callers :
> >
> > valgrind --num-callers=50 samba -i -M single
>
> Thanks for getting me that. I've managed to reproduce it here, but not
> under valgrind, and only when I hack the code to force a timeout. At
> least this should help me figure out why we process the winbind socket
> close, which is the crux of this issue.
I think I've found the cause of the issue you are hitting. There is
still another issue with the nested event loop in the krb5 libs, but
these two patches should help significantly.
As you have had more luck than I in reproducing this in a unaltered
setting, please let me know if this helps.
Patches are for git master, but may apply to 4.0 as well.
Kai, Metze:
In reading the code, I cannot see why the DNS server would not suffer
the same issue, if the DNS clients closed it's socket. Should we find a
more generic way to do this in service_stream, or should just duplicate
this? I don't think other servers hit the same issue as they are
currently 'blocking' in terms of the socket handler.
Thanks,
Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
>From df7c099be9366b0439f12d0924bd2192ad4888bd Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abart...@samba.org>
Date: Thu, 27 Jun 2013 11:27:03 +1000
Subject: [PATCH 1/2] service_stream: Log if the connection termination is
deferred or not
---
source4/smbd/service_stream.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/source4/smbd/service_stream.c b/source4/smbd/service_stream.c
index 22c4c04..74bb477 100644
--- a/source4/smbd/service_stream.c
+++ b/source4/smbd/service_stream.c
@@ -60,7 +60,11 @@ void stream_terminate_connection(struct stream_connection *srv_conn, const char
if (!reason) reason = "unknown reason";
- DEBUG(3,("Terminating connection - '%s'\n", reason));
+ if (srv_conn->processing) {
+ DEBUG(3,("Terminating connection deferred - '%s'\n", reason));
+ } else {
+ DEBUG(3,("Terminating connection - '%s'\n", reason));
+ }
srv_conn->terminate = reason;
--
1.7.11.7
>From 0daf694bce47710a62f7e38aa2830bc1b40f3dfc Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abart...@samba.org>
Date: Thu, 27 Jun 2013 11:28:03 +1000
Subject: [PATCH 2/2] s4-winbindd: Do not terminate a connection that is still
pending
Instead, wait until the call attempts to reply, and let it terminate then
(often this happens in the attempt to then write to the broken pipe).
Andrew Bartlett
---
source4/winbind/wb_samba3_protocol.c | 5 +++++
source4/winbind/wb_server.c | 14 +++++++++++++-
source4/winbind/wb_server.h | 5 ++++-
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/source4/winbind/wb_samba3_protocol.c b/source4/winbind/wb_samba3_protocol.c
index 2846e9c..1b78c99 100644
--- a/source4/winbind/wb_samba3_protocol.c
+++ b/source4/winbind/wb_samba3_protocol.c
@@ -297,6 +297,8 @@ NTSTATUS wbsrv_samba3_send_reply(struct wbsrv_samba3_call *call)
struct tevent_req *subreq;
NTSTATUS status;
+ call->wbconn->pending_calls--;
+
status = wbsrv_samba3_push_reply(call);
NT_STATUS_NOT_OK_RETURN(status);
@@ -355,9 +357,12 @@ NTSTATUS wbsrv_samba3_process(struct wbsrv_samba3_call *call)
return status;
}
+ call->wbconn->pending_calls++;
+
status = wbsrv_samba3_handle_call(call);
if (!NT_STATUS_IS_OK(status)) {
+ call->wbconn->pending_calls--;
talloc_free(call);
return status;
}
diff --git a/source4/winbind/wb_server.c b/source4/winbind/wb_server.c
index 983f9f5..fb67d23 100644
--- a/source4/winbind/wb_server.c
+++ b/source4/winbind/wb_server.c
@@ -31,7 +31,14 @@
void wbsrv_terminate_connection(struct wbsrv_connection *wbconn, const char *reason)
{
- stream_terminate_connection(wbconn->conn, reason);
+ if (wbconn->pending_calls == 0) {
+ char *full_reason = talloc_asprintf(wbconn, "wbsrv: %s", reason);
+ stream_terminate_connection(wbconn->conn, full_reason ? full_reason : reason);
+ } else {
+ DEBUG(3,("wbsrv: terminating connection due to '%s' defered due to %d pending calls\n",
+ reason, wbconn->pending_calls));
+ wbconn->terminate = reason;
+ }
}
static void wbsrv_call_loop(struct tevent_req *subreq)
@@ -41,6 +48,11 @@ static void wbsrv_call_loop(struct tevent_req *subreq)
struct wbsrv_samba3_call *call;
NTSTATUS status;
+ if (wbsrv_conn->terminate) {
+ wbsrv_terminate_connection(wbsrv_conn, wbsrv_conn->terminate);
+ return;
+ }
+
call = talloc_zero(wbsrv_conn, struct wbsrv_samba3_call);
if (call == NULL) {
wbsrv_terminate_connection(wbsrv_conn, "wbsrv_call_loop: "
diff --git a/source4/winbind/wb_server.h b/source4/winbind/wb_server.h
index 9b03004..941af68 100644
--- a/source4/winbind/wb_server.h
+++ b/source4/winbind/wb_server.h
@@ -94,9 +94,12 @@ struct wbsrv_connection {
/* storage for protocol specific data */
void *protocol_private_data;
- /* how many calls are pending */
+ /* how many calls are pending (do not terminate the connection with calls pending a reply) */
uint32_t pending_calls;
+ /* is this connection pending termination? If so, why? */
+ char *terminate;
+
struct tstream_context *tstream;
struct tevent_queue *send_queue;
--
1.7.11.7
--
To unsubscribe from this list go to the following URL and read the
instructions: https://lists.samba.org/mailman/options/samba
