[PATCH] Remove use of talloc_autofree_context() from source4/smbd/server.c

[Jeremy Allison]

On Tue, Apr 11, 2017 at 08:02:15AM +0200, Volker Lendecke wrote:
> On Mon, Apr 10, 2017 at 08:11:44PM -0700, Jeremy Allison via samba-technical wrote:
> > Whilst fixing the previous bug in reinitializing
> > s4 imessaging after fork(), I thought there should
> > be a way to move the /bin/samba event context off
> > the talloc_autofree context, which was causing all
> > the trouble.
> 
> Cool stuff :-)
> 
> Attached find a patchset with a few to-squash patches and a question.
> No formal review yet, but some minor cleanups (IMHO at least).

I'll post a new patchset + the squashed patches soon.

Here is the answer to your question:

---------------------------------------------------------------------
Subject: [PATCH 13/15] VL: I'd love to see why this is necessary. What fails
 without it?

I can see it's cleaner and the next commit (probably) builds upon it,
but the actual use case would be great to understand

s4: messaging: When talloc_free()'ing an event context, only remove msg_dgm_ref's that point to *that* context.
---------------------------------------------------------------------

Nothing fails without it and it's not strictly necessary.

I did it as a defensive programming change to make sure we
only talloc_free msg_dgm_ref from a event_context destructor
that is pointing to itself.

What made me think about this was I started thinking
about the nested event loop problem, and how we might
eventually remove all of these in the s4 code and turn
them into correctly embedded contexts as we do in smbd.

(Vague plan along those lines was at some point
to remove the call to tevent_loop_allow_nesting() and see what
crashes I get when running 'make test' :-).

If we did that, we might end up with multiple calls
to imessaging_init() from one process down the road.
The data structures inside source4/lib/messaging.c
contain a linked list of struct imessaging_context
pointers, and each of those has a possibly different
struct tevent_context pointer.

As I was messing with this areas it seemed like a
harmless thing to fix (and I knew if I didn't do
it I might forget later :-).

So defensively it might help in the future, but it's not
needed right now and doesn't prevent any crash. If you really
hate it and want to leave the current call to
imessaging_dgm_unref_all() alone I'm OK with that too.

Cheers,

	Jeremy