Hi,
On 2020-08-07 12:29:03 -0400, Robert Haas wrote:
> On Thu, Aug 6, 2020 at 11:46 PM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> > I sent the patch previously[1], but attaching here again, modifies
> > cancel_before_shmem_exit() function comment to reflect the safe usage
> > of before_shmem_exit_list callback mechanism and also removes the
> > point "For simplicity, only the latest entry can be removed*********"
> > as this gives a meaning that there is still scope for improvement in
> > cancel_before_shmem_exit() search mechanism.
> >
> > Thoughts?
>
> I think that the first part of the comment change you suggest is a
> good idea and would avoid developer confusion, but I think that the
> statement about unordered removal of comments being risky doesn't add
> much. It's too vague to help anybody and I don't think I believe it,
> either. So I suggest something more like:
>
> - * callback. For simplicity, only the latest entry can be
> - * removed. (We could work harder but there is no need for
> - * current uses.)
> + * callback. We only look at the latest entry for removal, as we
> + * expect the caller to use before_shmem_exit callback mechanism
> + * in the LIFO order.
In which situations is the removal actually useful *and* safe, with
these constraints? You'd have to have a very narrow set of functions
that are called while the exit hook is present, i.e. basically this
would only be usable for PG_ENSURE_ERROR_CLEANUP and nothing else. And
even there it seems like it's pretty easy to get into a situation where
it's not safe.
Greetings,
Andres Freund