[PATCH] Cache messaging dgm connections

[Jeremy Allison]

On Thu, Sep 15, 2016 at 07:08:17AM +0200, Ralph Böhme wrote:
> On Wed, Sep 14, 2016 at 10:01:56PM -0700, Jeremy Allison wrote:
> > On Thu, Sep 15, 2016 at 06:52:48AM +0200, Ralph Böhme wrote:
> > > Hi Garming,
> > > 
> > > On Thu, Sep 15, 2016 at 11:36:55AM +1200, Garming Sam wrote:
> > > > Seen some flapping tests in autobuild, and it looks like it's related to
> > > > the dgram changes. I originally thought it was another replication
> > > > issue, because of an IO timeout, but apparently not.
> > > > 
> > > > I'm not sure if it's the callers fault here, but I thought it would be
> > > > easier if you could take a look and give some advice.
> > > 
> > > sorry for this.
> > > 
> > > > https://git.samba.org/autobuild.flakey.sn-devel-144/2016-09-14-1112/samba.stderr
> > > 
> > > The problem is here:
> > > 
> > > #29 0x00002af8894bdd54 in dns_notify_dnssrv_done (req=0x2af892d1f170) at ../source4/dsdb/samdb/ldb_modules/dns_notify.c:72
> > > 
> > > This calls talloc_free() on an imessaging context and the crash is
> > > similar to what I was trying to fix in 02bb6b2: at the time a tevent
> > > timer destructor is called in the course of the talloc_free() on the
> > > imessaging context
> > > 
> > > #9  0x00002af88224ba1d in tevent_common_timed_destructor (te=0x2af88c8a4b70) at ../lib/tevent/tevent_timed.c:140
> > > 
> > > some required object is already freed. Still trying to understand
> > > which object, I don't think it's the tevent context like in 02bb6b2
> > > but it could be the same problem.
> > > 
> > > Still looking into it...
> > 
> > Yeah me too. I've been studying for a couple of
> > hours, and haven't found it yet :-). Not being
> > able to catch it under gdb is the hard part.
> > 
> > Let's see which of us finds it first :-).
> 
> appreciate your help, all this ref and handle stuff is really
> voodoo.

My money is on the event context getting freed with
active connections. I'm cheating here as this is
something metze already discovered with Volker's
tmsgd work :-).

Metze's patch for this is attached (modified to
compile in master). I'm trying a private autobuild
with it now.
-------------- next part --------------
commit 8c26d4706e6b144763b4cb8a5da25ac26e1035bc
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Sep 15 05:14:15 2016 +0200

    crash test
    
    Signed-off-by: Jeremy Allison <jra at samba.org>

diff --git a/lib/poll_funcs/poll_funcs_tevent.c b/lib/poll_funcs/poll_funcs_tevent.c
index 3059ebc..d45149f 100644
--- a/lib/poll_funcs/poll_funcs_tevent.c
+++ b/lib/poll_funcs/poll_funcs_tevent.c
@@ -474,6 +474,7 @@ struct poll_funcs *poll_funcs_init_tevent(TALLOC_CTX *mem_ctx)
 static int poll_funcs_state_destructor(struct poll_funcs_state *state)
 {
 	size_t num_watches = talloc_array_length(state->watches);
+	size_t num_contexts = talloc_array_length(state->contexts);
 	size_t i;
 	/*
 	 * Make sure the watches are cleared before the contexts. The watches
@@ -482,6 +483,9 @@ static int poll_funcs_state_destructor(struct poll_funcs_state *state)
 	for (i=0; i<num_watches; i++) {
 		TALLOC_FREE(state->watches[i]);
 	}
+	for (i=0; i<num_contexts; i++) {
+		TALLOC_FREE(state->contexts[i]);
+	}
 	return 0;
 }
 
@@ -523,7 +527,7 @@ static int poll_funcs_tevent_context_destructor(
 	struct poll_funcs_tevent_context *ctx);
 
 static struct poll_funcs_tevent_context *poll_funcs_tevent_context_new(
-	TALLOC_CTX *mem_ctx, struct poll_funcs_state *state,
+	struct poll_funcs_state *state,
 	struct tevent_context *ev, unsigned slot)
 {
 	struct poll_funcs_tevent_context *ctx;
@@ -531,7 +535,7 @@ static struct poll_funcs_tevent_context *poll_funcs_tevent_context_new(
 	size_t num_timeouts = talloc_array_length(state->timeouts);
 	size_t i;
 
-	ctx = talloc(mem_ctx, struct poll_funcs_tevent_context);
+	ctx = talloc(ev, struct poll_funcs_tevent_context);
 	if (ctx == NULL) {
 		return NULL;
 	}
@@ -631,7 +635,7 @@ void *poll_funcs_tevent_register(TALLOC_CTX *mem_ctx, struct poll_funcs *f,
 	}
 	if (state->contexts[slot] == NULL) {
 		state->contexts[slot] = poll_funcs_tevent_context_new(
-			state->contexts, state, ev, slot);
+			state, ev, slot);
 		if (state->contexts[slot] == NULL) {
 			goto fail;
 		}