Segmentation fault when binding string parameters

[paurkedal]

The following program fails with a segmentation fault on my end (Ubuntu 16.04, opam switch 4.04.0):

open Printf

module M = Mariadb.Blocking

let env var def =
  try Sys.getenv var
  with Not_found -> def

let or_die where = function
  | Ok r -> r
  | Error (i, e) -> ksprintf failwith "%s: (%d) %s" where i e

let connect () =
  M.connect
    ~host:(env "OCAML_MARIADB_HOST" "localhost")
    ~user:(env "OCAML_MARIADB_USER" "root")
    ~pass:(env "OCAML_MARIADB_PASS" "")
    ~db:(env "OCAML_MARIADB_DB" "mysql") ()

let test dbh =
  for i = 0 to 19999 do
    let n = Random.int (1 lsl Random.int 8) in
    let s = String.init n (fun i -> "ACGT".[Random.int 4]) in
    (*Printf.printf "%s\n%!" s;*)
    let stmt = M.prepare dbh "SELECT ?" |> or_die "prepare" in
    (match M.Stmt.execute stmt [|`String s|] |> or_die "execute" with
     | Some res ->
        assert (M.Res.num_rows res = 1);
        (match M.Res.fetch (module M.Row.Array) res |> or_die "fetch" with
         | None -> assert false
         | Some row -> assert (M.Field.string row.(0) = s))
     | None -> assert false);
    M.Stmt.close stmt |> or_die "close"
  done

let () = test (connect () |> or_die "connect")

I think I've tracked the issue down to the allocation of the buffer storing the parameter value. The documentation of Ctypes.allocate says that "the value will be automatically freed after no references to the pointer remain within the calling OCaml program", and the buffers are only stored in another allocated fragment, which I presume is not scanned by the ocaml garbage collector.

[paurkedal]

I may be wrong about the cause. I tried the following

diff --git a/lib/bind.ml b/lib/bind.ml
index 40d3d8c..754ddb4 100644
--- a/lib/bind.ml
+++ b/lib/bind.ml
@@ -9,6 +9,7 @@ type t =
   ; is_null : char ptr
   ; is_unsigned : char
   ; error : char ptr
+  ; mutable buffer : unit ptr option
   }

 type buffer_type =
@@ -72,6 +73,7 @@ let alloc count =
   ; is_null = allocate_n char ~count
   ; is_unsigned = no
   ; error = allocate_n char ~count
+  ; buffer = None
   }

 let bind b ~buffer ~size ~mysql_type ~unsigned ~at =
@@ -79,6 +81,7 @@ let bind b ~buffer ~size ~mysql_type ~unsigned ~at =
   let size = Unsigned.ULong.of_int size in
   let bp = b.bind +@ at in
   let lp = b.length +@ at in
+  b.buffer <- Some buffer;
   lp <-@ size;
   setf (!@bp) T.Bind.length lp;
   setf (!@bp) T.Bind.is_unsigned unsigned;

The above program now runs, but raising the number of iterations to 200000, it segfaults.

[andrenth]

Despite not fixing the issue (or maybe uncovering another one?) your patch seems correct. I'm applying it locally and investigating the segfault now.

On Sun, Dec 18, 2016 at 10:01 AM, Petter Urkedal notifications@github.com wrote:

I may be wrong about the cause. I tried the following

diff --git a/lib/bind.ml b/lib/bind.ml index 40d3d8c..754ddb4 100644 --- a/lib/bind.ml +++ b/lib/bind.ml @@ -9,6 +9,7 @@ type t = ; is_null : char ptr ; is_unsigned : char ; error : char ptr

• ; mutable buffer : unit ptr option }

  type buffer_type = @@ -72,6 +73,7 @@ let alloc count = ; is_null = allocate_n char ~count ; is_unsigned = no ; error = allocate_n char ~count

• ; buffer = None }

  let bind b ~buffer ~size ~mysql_type ~unsigned ~at = @@ -79,6 +81,7 @@ let bind b ~buffer ~size ~mysql_type ~unsigned ~at = let size = Unsigned.ULong.of_int size in let bp = b.bind +@ at in let lp = b.length +@ at in

• b.buffer <- Some buffer; lp <-@ size; setf (!@bp) T.Bind.length lp; setf (!@bp) T.Bind.is_unsigned unsigned;

The above program now runs, but raising the number of iterations to 200000, it segfaults.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/andrenth/ocaml-mariadb/issues/2#issuecomment-267817428, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB2BZLbVplkiS-VPCsctCtW5rBiToFQks5rJSCwgaJpZM4LQG9E .

[yallop]

The problem @paurkedal identifies (with references to fat pointers returned by allocate) comes up more often than I'd like, and I'm hoping to improve the situation in a future ctypes release. The issue https://github.com/ocamllabs/ocaml-ctypes/issues/476 will be updated once there's some progress.

[andrenth]

No luck so far, other than finding out that the problem isn't specific to string parameters... selecting an integer will end up passing the wrong value after a few thousand iterations too.

Will keep looking...

[andrenth]

I believe I have a fix. Your code works with no segfaults after 500k iterations here. I'll clean it up and make a commit tomorrow.

[andrenth]

@paurkedal can you check if the two most recent commits fix the problem for you?

[paurkedal]

Yes, also the caqti tests work now for the mariadb backend, which I expect to release as soon as I weed out my own bugs.

[andrenth]

Awesome. Do you mind if I add your code to the repository as a "stress test" for the library?

I'm almost ready to release a new version with this fix but I'm also wondering what to do about the libmariadbclient vs connector/c incompatibilities (see the other open issue)...

[paurkedal]

Sure, feel free to include my stress test.