vanishing messaging context names

[Andrew Bartlett]

On Tue, 2017-03-28 at 14:23 -0700, Jeremy Allison wrote:
> On Wed, Mar 29, 2017 at 10:09:16AM +1300, Andrew Bartlett via samba-
> technical wrote:
> > On Tue, 2017-03-28 at 12:08 -0700, Jeremy Allison wrote:
> > > On Wed, Mar 29, 2017 at 07:02:34AM +1300, Andrew Bartlett wrote:
> > > > On Tue, 2017-03-28 at 09:11 -0700, Jeremy Allison via samba-
> > > > technical
> > > > wrote:
> > > > > On Tue, Mar 28, 2017 at 01:20:03PM +0200, Andrew Bartlett
> > > > > wrote:
> > > > > 
> > > > > > commit a47a8e41bd3acc20d40ba78449d89775bcdd73ed
> > > > > > Author: Andrew Bartlett <abartlet at samba.org>
> > > > > > Date:   Fri Mar 24 13:07:06 2017 +1300
> > > > > > 
> > > > > >     samba-tool: Ensure that samba-tool processes --
> > > > > > name=not-
> > > > > > existing does not error
> > > > > >     
> > > > > >     Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> > > > > >     Reviewed-by: Garming Sam <garming at catalyst.net.nz>
> > > > > >     BUG: https://bugzilla.samba.org/show_bug.cgi?id=12705
> > > > > 
> > > > > I wish you'd discussed this one with me first before pushing
> > > > > it.
> > > > > 
> > > > > This is only masking a real bug I think. Without it, We can
> > > > > see
> > > > > that a bin/samba process is removing the "samba" name without
> > > > > ever having registered it. And we don't know why.
> > > > 
> > > > Looking over the code, last night for related reasons (trying
> > > > to
> > > > make
> > > > ldap multi-process again, so moving message_dgm cleanup around)
> > > > I
> > > > think
> > > > a talloc destructor is removing the name from the names
> > > > database in
> > > > a
> > > > child process after a fork().
> > > 
> > > Yes, looks like the NTVFS smb_server process is doing this.
> > > Still looking closely, but it looks plausible.
> > > 
> > > WHEN CAN WE REMOVE THIS CODE ? Please ? :-).
> > 
> > It isn't the fault of the NTVFS smb_server, we will hit the same
> > issue
> > when we make the LDAP server multi-process, or with the current
> > NETLOGON server being multi-process.
> 
> No, I already know that. It's just one less thing less
> to fix.
> 
> I really want to get rid of all uses of talloc_autofree_context(),
> it is almost tailor made to encourge this sort of sloppy
> lifetime management :-(.

I agree.  I've been bitten by it before. 

In any case, here is a patch.  The result of irpc_add_name() is no
longer removed by talloc_free(), but is removed when the stream or task
is terminated.  In source3 this is left as-was, but it is safe because
only the notify server registers a name.

Now we just need tests to show it stays around, that don't rely on
side-effects of other tests and test test order.

Andrew Bartlett
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-messaging-Move-names-cleanup-from-the-destructor-on-.patch
Type: text/x-patch
Size: 3595 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170329/43313c43/0001-messaging-Move-names-cleanup-from-the-destructor-on-.bin>