Make management of C-memory allocated from OCaml more explicit

[robhoes]

Ctypes has a number of functions that allocate C memory and return an OCaml value containing a pointer to it. Examples are make and CArray.make. This C memory is automatically freed when the associated OCaml value is collected by the GC due to the finaliser that is attached to it.

This is convenient, since it makes it harder for memory leaks to occur due to forgetting to free the memory. On the other hand, to avoid memory corruption, you need to be very careful to ensure that the OCaml value is kept alive (not GC-able) for as long as there is (C) code using the memory. It turns out that it is not always straightforward to do this, and it is easy to make mistakes.

An example is the use of a C function that takes a pointer to memory allocated from OCaml (as above), and returns a pointer to (somewhere in) the memory, perhaps after writing some data to it. The returned pointer will be wrapped in a new OCaml value, distinct from the original one, because ctypes can't tell that they are related. We must now keep the original value alive, along with the returned one.

A real example (thanks @yallop):

let strchr = Foreign.foreign "strchr" (ptr char @-> char @-> returning (ptr char)) in
let p = CArray.of_string "abc" in
let q =  strchr (CArray.start p) 'a' in
let () = Gc.compact () in
Printf.printf "%c\n" !@q

Immediately after strchr returns, value p is at risk of being GC'ed, which would free the array. From then on, q is pointing to invalid memory, and an arbitrary character may be printed in the last line. To avoid this, we would have to actually "use" p somehow after the last use of q, which is awkward and unintuitive, and therefore error prone.

See xapi-project/xen-api#3669 for a real-life example of a fix that was made for such a problem.

Something similar occurs when allocating memory from OCaml, and storing the pointer in a struct, even if the struct is also allocated from OCaml. It is mandatory to keep all OCaml values alive. See xapi-project/ocaml-opasswd#14 for an example of this, which is a fix that still turned out to be incomplete!

There are several other subtleties in the area of memory management, which I have not mentioned above, but which users of ctypes should be aware to avoid corruption. Such mistakes could be avoided by adopting a more explicit memory model, which does not use finalisers, and where it is up to the OCaml program to free the (C) memory using a call to a deallocation function (e.g. Ctypes.deallocate). The trade-off is indeed that doing so introduces a risk of memory leaks, but it may be clearer to see what is going on.

A possible way to do this, while maintaining backwards compatibility, is to add an optional, Boolean argument to all "make" functions, which has the effect that no finaliser is added to the new value.

[Fizzixnerd]

Following the discussion on: https://discuss.ocaml.org/t/blog-ocaml-ffi-sharp-edges-and-how-to-avoid-them/14935 :

@edwintorok made the suggestion of using a construct like ignore (Sys.opaque_identity x) as a method of keeping x alive at least until the execution of that statement with little-to-no runtime overhead.

I would like to open a small PR to add let keep_alive x = ignore (Sys.opaque_identity x), as in my testing it has been extremely effective at solving this problem.

The code above would transform to:

let strchr = Foreign.foreign "strchr" (ptr char @-> char @-> returning (ptr char)) in
let p = CArray.of_string "abc" in
let q =  strchr (CArray.start p) 'a' in
let () = Gc.compact () in
let () = keep_alive (p, q)
Printf.printf "%c\n" !@q

Where I have kept q alive just for simplicity's sake as well, even though it's technically not required. This also demonstrates that you can happily just stick all the variables you need alive in a tuple.

I would suggest the following changes (and am willing to complete the work required for them):

1. Add keep_alive as defined above as a helper for users of Ctypes.
2. Add documentation for keep_alive where appropriate, likely somewhere near the top of the .mli or near allocate_n/allocate, as this is a pretty endemic issue it seems.
3. (Optional) Add an PPX that inserts keep_alive for you where appropriate. That is, the above code would be transformed to something that looks like:

let strchr = Foreign.foreign "strchr" (ptr char @-> char @-> returning (ptr char)) in
let%keep_alive p = CArray.of_string "abc" in
let%keep_alive q =  strchr (CArray.start p) 'a' in
let () = Gc.compact () in
Printf.printf "%c\n" !@q

which would transform roughly to:

let strchr = Foreign.foreign "strchr" (ptr char @-> char @-> returning (ptr char)) in
let p = CArray.of_string "abc" in
    let __gensym0 = 
        let q =  strchr (CArray.start p) 'a' in
            let __gensym1 = 
                let () = Gc.compact () in
                Printf.printf "%c\n" !@q
           in Ctypes.keep_alive q; __gensym1
    in Ctypes.keep_alive p; __gensym0

or similar.

If you don't want to muddy the waters with a PPX, I totally get it, and in that case would probably just create a separate opam package containing just that PPX. I do strongly think that the keep_alive function belongs in Ctypes, however.

[yallop]

Exposing a keep_alive function is a good idea, but I'm not sure that Sys.opaque_identity actually provides the necessary guarantees. So I recommend either

• guaranteeing in the OCaml distribution (perhaps just by updating the documentation) that calling Sys.opaque_identity will actually keep the object alive, or
• basing keep_alive on Ctypes_memory_stubs.use_value instead

[Fizzixnerd]

See ocaml/ocaml#13288 for updates, but it should work according to the above linked information and ocaml/ocaml#9412 for when the change occured.

[yallop]

Ah, indeed: after ocaml/ocaml#9412 Sys.opaque_identity can be used to guarantee liveness, so a patch implementing keep_alive that way would be welcome. (And if you're so inclined, replacing Ctypes_memory_stubs.use_value with your new function in the ctypes implementation would be welcome, too.)

[gasche]

I don't think you need a ppx for this:

(* library code *)
let alive x f =
  let result = f x in
  Sys.opaque_identity x;
  result

(* generic binding operator *)
let (let@) x f = f x

(* your example *)
let@ p = CArray.of_string "abc" |> alive in
let@ q = strchr (CArray.start p) 'a' |> alive in
let () = Gc.compact () in
Printf.printf "%c\n" !@q

(But: I agree that this programming model looks fairly fragile overall, almost worse than writing the C code directly. I wonder what's the desirable big-picture for Ctypes.)