[Date Prev][Date Next]
Re: (ITS#5340) REP_ENTRY_MODIFIABLE bug in dynlist
> it is not
> clear what happens when a callback chain is interrupted by
> slap_null_cb() or similar, without getting to slap_send_search_entry().
> This seems to indicate that callbacks should always provide a last
> resort means to release the resources they set; if read-only, by keeping
> track of what they sent; if modifiable, by freeing the contents of
> rs->sr_* if not NULL, setting it to NULL to prevent further cleanup.
That sounds cumbersome, I hope slapd could take care of that somehow.
But I don't see how the be_release() code can work now. It sounds like
be->be_release() functions must check (how?) that the entry was created
by 'be', and otherwise pass it on to the next overlay/backend or
otherwise to entry_free(). Might involve mucking with op->o_bd and
sr_entry->e_private, I suppose. Except maybe I'm missing some existing
magic since slapd doesn't regularly crash...
be->be_release() does receive entries that were not created by 'be' or
at least not with be->be_fetch(), see openldap-devel thread 'slapd API'
in mar 2008.
> Similarly, the existence of REP_ENTRY_MUSTBEFREED is not totally clear:
> in principle as soon as REP_ENTRY_MODIFYABLE is set, it should imply
> REP_ENTRY_MUSTBEFREED; the only difference in the semantics of the two
> is that REP_ENTRY_MODIFYABLE without REP_ENTRY_MUSTBEFREED implies that
> the callback that set the former will take care of freeing the entry;
> however, other callbacks may further modify it, so freeing temporary
> data should probably be left to the final handler.
That's not my impression. MODIFIABLE would be that other modules than
the creator can modify the entry - but the creator might still be the
one who will free it. MUSTBEFREED is that the entry must be
entry_free()ed - the creator will not do it (or not do it unless that
flag is set).
So, if I'm getting this right...
A backend must expect an entry to change or be freed if it sends the
entry with REP_ENTRY_<MUSTRELEASE, MUSTBEFREED or MODIFIABLE>, or if
it passes through a slap_callback.sc_response.
back-ldif does not: it uses e_name/e_nname _after_ sending with with
Nor overlay retcode, it calls retcode_entry_response(,,,rs->sr_entry)
which makes pointers into sr_entry without checking if those flags are
set. If I'm getting that code correctly. I haven't tested.
Others apparent problems (also not tested, I've just browsed the code):
Overlays that obey and reset MUSTBEFREED or MUSTRELEASE, do not
necessarily clear or set MODIFIABLE when setting a new entry.
translucent does not, even though it is a careful one which
obeys both MUSTBEFREED and MUSTRELEASE.
I'm not sure when the code must clear the entry_related flags.
Some code which sets new entries seem to assume they are zero,
but some code which sets sr_entry=NULL does not clear the flags.
There are sr_flags bits which are not about the entry, in particular
REP_MATCHED_MUSTBEFREED and REP_MATCHED_MUSTBEFREED. Looks like these
flags can get lost when but some code sets sr_flags = <something>
rather sr_flags |= <something> or sr_flags ~= ~<something>.