Custom blocks and finalization

Custom blocks and finalization

[Markus Mottl]

Hi,

we've recently run into a class of bugs concerning finalization
functions with custom blocks that is probably not uncommon in OCaml
bindings (e.g. Postgresql-bindings, SSL-bindings, likely others).  It
seems somewhat unintuitive, but finalizers registered from within C do
_not_ behave the same as ones registered from OCaml-code with the
Gc-module.

The OCaml manual explicitly states that custom functions registered
from within C are not allowed to allocate values, perform callbacks,
(de)register roots (I guess global roots, too?), or (as is obviously
also true) release/acquire the OCaml-runtime lock.  Developers
probably often miss this information.  The consequences of bugs
violating any of these constraints are usually rare and hard to
understand segfaults.

This means that people will have to register finalizers from within
OCaml for such values, which is certainly rather inconvenient.  As a
general rule, it seems advisable to never use finalizers that need to
release the runtime lock, e.g. if finalization code can block.  The
reason is that finalizers can be run from any thread, including ones
that should not block for an indefinite amount of time.  This implies
that people who have designed APIs using finalization that way should
consider redesigning it such that users have to "clean up" manually,
thus forcing them to think about when and in which thread this
happens.

Otherwise, it might be helpful if the OCaml-team could consider
whether this situation can be improved.  For example not being allowed
to register/unregister roots to values seems overly restrictive, since
global roots (e.g. for protecting OCaml-callback closures) sometimes
need to be associated with C-values (e.g. for allowing some external
C-library to efficiently perform callbacks into OCaml).  Registering
the finalizer from within C during allocation rather than having to
wrap up the value a second time with an OCaml-finalizer would seem
much simpler.

Regards,
Markus

-- 
Markus Mottl        http://www.ocaml.info        markus.mottl@gmail.com

Re: [Caml-list] Custom blocks and finalization

[Dr. Thomas Fischbacher]

Markus Mottl wrote:
> Otherwise, it might be helpful if the OCaml-team could consider
> whether this situation can be improved.  For example not being allowed
> to register/unregister roots to values seems overly restrictive, since
> global roots (e.g. for protecting OCaml-callback closures) sometimes
> need to be associated with C-values (e.g. for allowing some external
> C-library to efficiently perform callbacks into OCaml).  

I would just like to point out that the OCaml/Python based "nmag"
(/"nsim") MPI-parallel micromagnetic simulator does this a lot, and
it is actually needed for a seamless vice-versa integration of
Python/Caml closures (i.e. one can freely call Python closures
from within OCaml, and OCaml closures from within Python.)

I think we could change this in such a way that registering the
finalizer is done by a detour through OCaml wherever the
problem arises, but it just feels very wrong to do so, somehow...

-- 
best regards,
Thomas Fischbacher
t.fischbacher@soton.ac.uk

Re: [Caml-list] Custom blocks and finalization

[Markus Mottl]

On Fri, May 1, 2009 at 10:18, Dr. Thomas Fischbacher
<t.fischbacher@soton.ac.uk> > Can you provide some more information on
the low level
> details about this issue? E.g. can it arise in non-threaded
> OCaml code?

Yes, if your C-finalizer e.g. needs to remove global roots, this may
not be sound.  I am not totally sure this is actually true in the
current implementation, but at least the documentation clearly
indicates that any interaction with the OCaml-runtime from within
C-finalizers is prohibited.  You may essentially just extract C-values
/ pointers from the OCaml-value to be reclaimed and manipulate those.

> If there is any way to get this resolved by e.g. introducing
> another set of C-level functions within OCaml and asking
> people to switch over to using those, I would consider
> that the most appropriate way forward.

It's my superficial impression that the runtime system could be fixed
such that interactions with it from within C-finalizers and other
custom functions becomes safe, maybe at the expense of some
performance.  E.g. if you register finalizers with the Gc-module from
within OCaml, it seems that the runtime first records all finalizer
functions that need to be called after a collection in a table.  Maybe
something similar can be done for C-finalizers?

Regards,
Markus

-- 
Markus Mottl        http://www.ocaml.info        markus.mottl@gmail.com

Re: [Caml-list] Custom blocks and finalization

[Dr. Thomas Fischbacher]

Markus Mottl wrote:

> Yes, if your C-finalizer e.g. needs to remove global roots, this may
> not be sound.  I am not totally sure this is actually true in the
> current implementation, but at least the documentation clearly
> indicates that any interaction with the OCaml-runtime from within
> C-finalizers is prohibited.  You may essentially just extract C-values
> / pointers from the OCaml-value to be reclaimed and manipulate those.
>   

I take it you are referring to this part of the documentation (18.9.1 
bottom):

===>
Note: the finalize, compare, hash, serialize and deserialize functions
attached to custom block descriptors must never trigger a garbage 
collection.
Within these functions, do not call any of the Caml allocation 
functions, and
do not perform a callback into Caml code. Do not use CAMLparam to 
register the
parameters to these functions, and do not use CAMLreturn to return the 
result.
<===

Let's have a look at byterun/globroots.c:

/* Un-register a global C root */

CAMLexport void caml_remove_global_root(value *r)
{
  struct global_root * update[NUM_LEVELS];
  struct global_root * e, * f;
  int i;

  /* Init "cursor" to list head */
  e = (struct global_root *) &caml_global_roots;
  /* Find element in list */
  for (i = caml_global_roots.level; i >= 0; i--) {
    while (1) {
      f = e->forward[i];
      if (f == NULL || f->root >= r) break;
      e = f;
    }
    update[i] = e;
  }
  e = e->forward[0];
  /* If not found, nothing to do */
  if (e == NULL || e->root != r) return;
  /* Rebuild list without node */
  for (i = 0; i <= caml_global_roots.level; i++) {
    if (update[i]->forward[i] == e)
      update[i]->forward[i] = e->forward[i];
  }
  /* Reclaim list element */
  caml_stat_free(e);
  /* Down-correct list level */
  while (caml_global_roots.level > 0 &&
         caml_global_roots.forward[caml_global_roots.level] == NULL)
    caml_global_roots.level--;
}

...and memory.c says:

CAMLexport void caml_stat_free (void * blk)
{
  free (blk);
}



So... how could caml_remove_global_root() actually trigger a heap GC?

-- 
best regards,
Thomas Fischbacher
t.fischbacher@soton.ac.uk

Re: [Caml-list] Custom blocks and finalization

[Markus Mottl]

On Wed, May 6, 2009 at 12:30, Dr. Thomas Fischbacher
<t.fischbacher@soton.ac.uk> > I take it you are referring to this part
of the documentation (18.9.1
> bottom):
>
> ===>
[snip]

Yes.

> Do not use CAMLparam to register
> the
> parameters to these functions, and do not use CAMLreturn to return the
> result.

Neither of the above will ever trigger a garbage collection, but they
are still prohibited.  I don't know why these restrictions are
required, but according to the documentation it seems dangerous to
manipulate any runtime datastructures while in a finalizer.

> Let's have a look at byterun/globroots.c:
[snip]
> So... how could caml_remove_global_root() actually trigger a heap GC?

It certainly doesn't, but it obviously manipulates runtime
datastructures which may not necessarily be in a state during
finalization where this is safe.  I'd have to study the runtime code
in detail to learn more, but maybe the OCaml team can clarify this
issue more quickly?

Regards,
Markus

-- 
Markus Mottl        http://www.ocaml.info        markus.mottl@gmail.com

Re: [Caml-list] Custom blocks and finalization

[Dr. Thomas Fischbacher]

Markus,

> Neither of the above will ever trigger a garbage collection, but they
> are still prohibited.
So, I ask: Why? Unregistering a global root evidently does not trigger a GC,
and starting a GC from within a C-registered custom
hasher/finalizer/comparer/serializer/etc is the one specific thing that
according to the docs causes trouble, so I don't really see why I should go
through quite a fair bit of our codebase to come up with a fix to something
which as far as I can see must not be a problem.

> It certainly doesn't, but it obviously manipulates runtime
> datastructures which may not necessarily be in a state during
> finalization where this is safe.  I'd have to study the runtime code
> in detail to learn more, but maybe the OCaml team can clarify this
> issue more quickly?
>   

Yes, clarification would be much appreciated. I still maintain that my
use of caml_remove_global_root() in a finalizer is not in violation of 
this spec:

===>
  Note: the finalize, compare, hash, serialize and deserialize functions
attached to custom block descriptors must never trigger a garbage 
collection.
Within these functions, do not call any of the Caml allocation 
functions, and
do not perform a callback into Caml code. Do not use CAMLparam to 
register the
parameters to these functions, and do not use CAMLreturn to return the 
result.
<===

So, if there is a problem with this practice, it is certainly an OCaml 
bug (i.e. spec
not matching behaviour) and should be fixed by the OCaml team.

-- 
best regards,
Thomas Fischbacher
t.fischbacher@soton.ac.uk

Re: [Caml-list] Custom blocks and finalization

[Xavier Leroy]

Markus Mottl wrote:

>> Let's have a look at byterun/globroots.c:
> [snip]
>> So... how could caml_remove_global_root() actually trigger a heap GC?
> 
> It certainly doesn't, but it obviously manipulates runtime
> datastructures which may not necessarily be in a state during
> finalization where this is safe.  I'd have to study the runtime code
> in detail to learn more, but maybe the OCaml team can clarify this
> issue more quickly?

I foresee absolutely no problems with registering/unregistering global
roots from a C finalizer.  As the manual states, the big no-no in
C functions attached to custom blocks is allocating in the heap,
either directly or via a callback into Caml or by releasing the global
lock.  Within a finalizer, you should also refrain from raising an
exception, as this would leave the GC is a bizarre state.  But global
roots operations are OK.

As for not using the CAMLparam, etc macros in these functions: since
these functions must not allocate, the macros are certainly not
needed.  I don't think that actually using them could cause a problem,
but that would be silly anyway, so don't use them in this context.

Hope this answers your question.

- Xavier Leroy

Re: [Caml-list] Custom blocks and finalization

[Markus Mottl]

On Fri, May 8, 2009 at 05:36, Xavier Leroy <Xavier.Leroy@inria.fr> wrote:
> I foresee absolutely no problems with registering/unregistering global
> roots from a C finalizer.  As the manual states, the big no-no in
> C functions attached to custom blocks is allocating in the heap,
> either directly or via a callback into Caml or by releasing the global
> lock.  Within a finalizer, you should also refrain from raising an
> exception, as this would leave the GC is a bizarre state.  But global
> roots operations are OK.

Thanks, good to know.  I had only taken a superficial look at the
implementation and didn't see any problem with global roots, but even
if this might be true now, I wasn't sure whether this would remain so
in the future.

> As for not using the CAMLparam, etc macros in these functions: since
> these functions must not allocate, the macros are certainly not
> needed.  I don't think that actually using them could cause a problem,
> but that would be silly anyway, so don't use them in this context.

Right, since allocations are prohibited anyway, one doesn't need to
protect the passed value and won't need local roots.  I guess the
documentation could still be clarified in this respect, because the
explicit prohibition of using the local root (de)registration
functions suggests that these operations are not guaranteed to be
sound and that this might also extend to global roots.

Regards,
Markus

-- 
Markus Mottl        http://www.ocaml.info        markus.mottl@gmail.com