talloc_tos() Re: 617ec07 ?

[Stefan (metze) Metzmacher]

Am 11.08.2010 08:45, schrieb Volker Lendecke:
> On Wed, Aug 11, 2010 at 04:27:00PM +1000, tridge at samba.org wrote:
>> I should start off by saying that I think talloc_tos() should
>> definately be in common code. It is now used in so much s3 code that
>> we have to support it.
>>
>> My main confusion with talloc_tos() comes when think about how it
>> interacts with async events. With a lot of what happens in Samba now
>> happening in callbacks, it isn't clear to me that a stack based
>> approach is a good match.
>>
>> Is there some way that talloc_tos and events is handled specially so
>> it makes sense? For example, what happens to memory allocated in timer
>> or signal callbacks? What about protocol callbacks on sockets?
>>
>> It may well be handled fine, I don't know of a specific bug related to
>> talloc_tos and events, I'm just flagging it as a combination that
>> leaves me a bit puzzled.
> 
> The rule is pretty simple: Once you leave a routine with
> return or at the end of a callback, talloc_tos() will be
> automatically be freed. I see it like a C++ destructor that
> can be postponed by a few instructions. So for anything that
> needs to survive the current callback, talloc_tos() can not
> be used. 

The memory from talloc_tos() is only valid within the scope of the
current block/function. Like variables on the stack. So everywhere
you could also use a fixed length stack variable you could use
talloc_tos().

The simplest way of adding support for talloc_tos() would be to have
tevent hooks at the beginning and the end of tevent_loop_once(),
which would call talloc_stackframe() and talloc_free().

For new async code we should have this:

static struct tevent_req *rpccli_bh_raw_call_send(TALLOC_CTX *mem_ctx,
»·······»·······»·······»·······»·······»·······  struct tevent_context *ev,
»·······»·······»·······»·······»·······»·······  struct
dcerpc_binding_handle *h,
»·······»·······»·······»·······»·······»·······  const struct GUID *object,
»·······»·······»·······»·······»·······»·······  uint32_t opnum,
»·······»·······»·······»·······»·······»·······  uint32_t in_flags,
»·······»·······»·······»·······»·······»·······  const uint8_t *in_data,
»·······»·······»·······»·······»·······»·······  size_t in_length)
{
»·······struct tevent_req *req;
»·······struct rpccli_bh_raw_call_state *state;

»·······req = tevent_req_create(mem_ctx, &state,
»·······»·······»·······»·······struct rpccli_bh_raw_call_state);
»·······if (req == NULL) {
»·······»·······return NULL;
»·······}

where state is internal state for the whole async call from the _send to
the _recv
functions.

By default 'state' should be used for all allocations which belong to
the async call. This makes the cleanup also easy, everything that needs
to be exposed to the caller should talloc_moved() to the callers context
in the _recv function.

static NTSTATUS rpccli_bh_raw_call_recv(struct tevent_req *req,
»·······»·······»·······»·······»·······TALLOC_CTX *mem_ctx,
»·······»·······»·······»·······»·······uint8_t **out_data,
»·······»·······»·······»·······»·······size_t *out_length,
»·······»·······»·······»·······»·······uint32_t *out_flags)
{
»·······struct rpccli_bh_raw_call_state *state =
»·······»·······tevent_req_data(req,
»·······»·······struct rpccli_bh_raw_call_state);
»·······NTSTATUS status;

»·······if (tevent_req_is_nterror(req, &status)) {
»·······»·······tevent_req_received(req);
»·······»·······return status;
»·······}

»·······*out_data = talloc_move(mem_ctx, &state->out_data.data);
»·······*out_length = state->out_data.length;
»·······*out_flags = state->out_flags;
»·······tevent_req_received(req);
»·······return NT_STATUS_OK;
}

tevent_req_received(req) frees 'state' and marks the 'req' as
TEVENT_REQ_RECEIVED.
The caller needs to free 'req' as it represents the external state of
the async call.

metze


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100811/ba4dfc50/attachment-0001.pgp>