[Patches] Preparation for tevent impersonation (part1)
ddiss at samba.org
Fri Jun 15 12:07:26 UTC 2018
On Fri, 15 Jun 2018 09:25:08 +0200, Volker Lendecke wrote:
> On Thu, Jun 14, 2018 at 11:54:50PM +0200, David Disseldorp wrote:
> > On Thu, 14 Jun 2018 20:37:37 +0200, Volker Lendecke wrote:
> > > > Talloc stackframe handling is broken in many of places, unfortunately.
> > > > Continuing to use it for event based async code will only lead to more
> > > > problems in future IMO.
> > >
> > > What do you propose instead?
> > In most async cases we already have a memory context for the request
> > state, which makes a perfectly adequate parent for short-lived
> > tallocations. It may mean passing mem_ctx arguments around a bit more,
> > but is IMO far less error prone and easier to follow compared to random
> > usage of the talloc_tos() global and subsequent garbage collection.
> Manually passing down temporary contexts is just as error-prone: It's
> too easy to accidentially pass down a long-term context where a
> short-lived one is needed. For me talloc_tos() is a clear indication
> that a short-lived one is requested. The guarantees for talloc_tos()
> are that it lives as long as the current function runs and that it
> will be cleaned up "quickly" after the function returned. Clear and
> concise, at least to me.
I disagree. For short-lived heap-backed allocations, creating a new
talloc child under a longer-term context and subsequently freeing it
allows one to easily determine the lifecycle of the variable. This is
not the case for talloc_tos() allocations without analysing the state of
the talloc stackframe through the entire call chain - "quickly" is
A function returning memory allocated on the current stackframe without
taking a talloc_tos() parameter is evil IMO, as it's completely unclear
from the caller where the memory came from.
> I haven't followed precisely, but f4f3abfa0e18bb just committed might
> be a good example of what can go wrong if you explicitly pass around
> meant-to-be-short-lived contexts explicitly.
Short of moving to Rust or Go, I don't think we'll ever be completely
free of memory leaks. talloc_tos() does help reduce the impact of some
memory leaks, but the drawbacks far outweigh the benefits IMO.
> If talloc_tos() is no longer en vogue, we need a similarly concise
In talloc we already have a very nice hierarchical allocator which,
alongside the stack, is perfectly capable of providing short lived
More information about the samba-technical