TALLOC: Discussion of destructor processing

[Stefan (metze) Metzmacher]

Hi,

a return of -1 from a destructor doesn't mean it "failed",
it means that it has a very good reason to reject a talloc_free() of all
it's children
and the free() on itself. It's the duty of the destructor logic to
prevent memory leaks!

Typically the caller doesn't need to do need to check the return value
of talloc_free(),
as from the caller's perspective the memory is gone and it should not
care if the destructor
wants to do anything special. The case where checking the return of
talloc_free() make sense
is when the caller exactly knows what the specific destructor is doing.

In samba we have a few places where we rely on that feature, e.g. see
tevent_common_timed_deny_destructor(), dcerpc_connection_destructor(),
packet_destructor(), smb2srv_request_deny_destructor(),
lpcfg_destructor(), system_session_destructor()

store_destructor in paged_results.c seems to be a bug, I think there's
no reason
to return -1 there. The same for db_ctdb_transaction_destructor().

Any complex operation that can fail and should be avoided and
a failing logic inside a destructor should not result in a return of -1.

The tstream_cli_np_destructor() example Volker mentioned, is in its
current state
because it was like that before. The correct implementation would trigger an
async close, let the tstream_cli_np become a talloc child of cli_state,
set the destructor to NULL and return -1. The callback function triggered
by the async close can call talloc_free() without destructor then.
Or it gets free'ed by a talloc_free(cli_state) by a higher level caller.

I think the only thing we should do here is to enhance the documentation.

And if you don't want to care about all this in your application, just only
use simple destructors and always return 0.

metze