Musings on talloc and talloc_free

[Sam Liddicott]

Some time last year when I got into trouble mixing talloc_free with
talloc_reference and have spent a lot of time trying to understand the
problem and solutions.

It seems like the real harm is not in mixing talloc_free and
talloc_reference, but in mixing talloc_free and talloc_steal, as uses of
talloc_free and talloc_steal depend implicitly on "knowing" the existing
owner.

If talloc_free() is called after talloc_steal() the caller should know
(have known) that the talloc_steal() did occur, and from the context
ought to be acting as the new owner.

Also consider an allocation with some talloc_references(). If
talloc_steal() is called after talloc_free() it won't be simple to
predict which reference-become-owner has been displaced, and the new
owner might have a shorter lifetime...

I now try to make usage rules based on these observations:

It is safe to call talloc_free if the actual owner is the supposed owner
(of course) which can only be certainly true if
a. talloc free has not been called before
b. if talloc_steal has been used, that the code calling talloc_free is
"aware" of this and acting for the new owner.

It is safe to use talloc_steal if the actual owner is the supposed owner
which can only be certainly true if:
a. talloc_free has not been used before - or it becomes indeterministic
who the new owner is
b. talloc_steal has not been called before, or if it has, the current
owner is known, or it is expecting to lose ownership.
[Of course internally talloc_steal can still be used internally as
owners are free'd and references become owners]

To me this shows that there is a lot of implicit context where
talloc_steal and talloc_free are used, in that the author "knows" it to
be safe.

Explicit use of talloc free can occur for:

1. short lived allocations (that maybe are never owned by non-NULL
context) where:
1a. any intermediate references that may have existed are gone by the
time talloc_free is called.
1b. the allocation has not been talloc_steal()'d

2. long lived allocations where
2a. the owner will not be free'd yet but it has finished with the
allocation and any other need has been talloc_reference()'d
2b. the allocation has not been talloc_steal()'d
2c. the owner is not depending on the destructor to be called to remove
the allocation from any DLIST or other collection, the failure of which
(due to references) will bring the allocation again to the attention of
the owner for another talloc_free (or other ugly situation a la: Vinny,
I thought you'd taken care of him)

This suggests that:
1. talloc free should never be called more than once per allocation,
because it becomes to hard to be sure who the second owner is.
2. that talloc_steal should never happen secretly but be a documented
part of whatever function invokes talloc_steal() as part of it's operation.
3. talloc_steal should never be directly called after a talloc_free (OK
internal to talloc)
4. When talloc_free is called after talloc_steal it had better be in the
context of the new owner - to be safe, why not just use talloc_unlink to
prove it?
and I also note
5. talloc_steal might replace a talloc_reference inside another function
followed by talloc_free by the caller.


This leaves interesting recommendations:

1. talloc_free (when not invoked internally) should raise an error if
called more than once on the same allocation. Any code that does this
"legitimately" because it "knows" who the second reference is, is both
bad, brittle and lazy, and should be using talloc_unlink.
(talloc_unlink or talloc_unreference count as talloc_free when the
reference is the current owner)

2. talloc_unlink() should abort when called with a bogus owner.

3. talloc_steal should raise an error if talloc_free has ever been
called (because it can't be sure who is being stolen from)

and as a tip to coders:

Try to not need to manually free allocations. If you need to, and if the
allocation doesn't have a short (1 function +optional callback) or
simple life (guaranteed no references or stealing), talloc_free should
probably not be used, but rather talloc_unlink - passing the supposed
owner as proof of good faith.

Sam