Fix memory alloc in caml_sqlite3_backup_init()

[mtelvers]

This PR addresses the segmentation fault exception in Sqlite3.Backup.init at line 1677 Sqlite3_backup_val(v_res) = res;

[pirbo]

I hit the same problem! But I worry that your patch is not GC safe and would suggest to instead do

diff --git a/src/sqlite3_stubs.c b/src/sqlite3_stubs.c
index 38092c9..b38dec1 100644
--- a/src/sqlite3_stubs.c
+++ b/src/sqlite3_stubs.c
@@ -180,7 +180,6 @@ static inline void maybe_raise_user_exception(int rc)
 
 #define Sqlite3_val(x) (*((db_wrap **) Data_custom_val(x)))
 #define Sqlite3_stmtw_val(x) (*((stmt_wrap **) Data_custom_val(x)))
-#define Sqlite3_backup_val(x) (*((sqlite3_backup **) Data_custom_val(x)))
 
 
 /* Exceptions */
@@ -1643,6 +1642,7 @@ CAMLprim value caml_sqlite3_changes_bc(value v_db)
 
 
 /* Backup functionality */
+#define Sqlite3_backup_val(x) (*((sqlite3_backup **) Data_abstract_val(x)))
 
 CAMLprim value caml_sqlite3_backup_init(
     value v_dst, value v_dst_name, value v_src, value v_src_name)
@@ -1674,6 +1674,7 @@ CAMLprim value caml_sqlite3_backup_init(
 
   if (NULL == res) raise_sqlite3_current(dst->db, "backup_init");
 
+  v_res = caml_alloc(1,Abstract_tag);
   Sqlite3_backup_val(v_res) = res;
 
   CAMLreturn(v_res);

[pirbo]

Apparently, this is not enough... It still segfault for me but this time the gdb backtrace is more mysterious

#0  0x0000555556cfa610 in ?? ()
#1  0x00007ffff7edd48e in ?? () from /lib/x86_64-linux-gnu/libsqlite3.so.0
#2  0x00007ffff7eea20a in sqlite3_backup_step () from /lib/x86_64-linux-gnu/libsqlite3.so.0
#3  0x00005555564ad8e3 in caml_sqlite3_backup_step (v_backup=<optimized out>, pagecount=5) at sqlite3_stubs.c:1691
#4  0x00005555561c6039 in camlSqlite3__anon_fn_2259 () at vendors/sqlite3.5.1.0/src/sqlite3.ml:433

[mtelvers]

For my application, sqlite3-backup, either version works. Is there something in your use case which is different to mine?

[pirbo]

@klakplok saved me! The problem is that Ocaml GC mustn't garbage collect the src and dst databases as long as the backup object is alive! (And because I forgot to close the dst database in my code there were no more reference to it after the backup_init and it was GCed...) His suggestion is to do (either on the Ocaml side or the C side)

diff --git a/src/sqlite3.ml b/src/sqlite3.ml
index e835d67..c2781da 100644
--- a/src/sqlite3.ml
+++ b/src/sqlite3.ml
@@ -519,24 +519,42 @@ module Aggregate = struct
 end  (* Aggregate *)
 
 module Backup = struct
-  type t
+  module Raw = struct
+    type t
 
-  external init :
-    dst : db -> dst_name : string ->
-    src : db -> src_name : string -> t = "caml_sqlite3_backup_init"
+    external init :
+      dst : db -> dst_name : string ->
+      src : db -> src_name : string -> t = "caml_sqlite3_backup_init"
 
-  external step : t -> (int [@untagged]) -> Rc.t
-    = "caml_sqlite3_backup_step_bc" "caml_sqlite3_backup_step"
+    external step : t -> (int [@untagged]) -> Rc.t
+      = "caml_sqlite3_backup_step_bc" "caml_sqlite3_backup_step"
 
-  external finish : t -> Rc.t = "caml_sqlite3_backup_finish"
+    external finish : t -> Rc.t = "caml_sqlite3_backup_finish"
 
-  external remaining : t -> (int [@untagged])
-    = "caml_sqlite3_backup_remaining_bc" "caml_sqlite3_backup_remaining"
+    external remaining : t -> (int [@untagged])
+      = "caml_sqlite3_backup_remaining_bc" "caml_sqlite3_backup_remaining"
     [@@noalloc]
 
-  external pagecount : t -> (int [@untagged])
-    = "caml_sqlite3_backup_pagecount_bc" "caml_sqlite3_backup_pagecount"
+    external pagecount : t -> (int [@untagged])
+      = "caml_sqlite3_backup_pagecount_bc" "caml_sqlite3_backup_pagecount"
     [@@noalloc]
+  end
+
+  type t = (Raw.t*db*db)
+  (* the databases mustn't be garbaged collected before the backup
+     object (which have references to them) so we bind their lifetime
+     together. *)
+
+  let init ~dst ~dst_name ~src ~src_name =
+    (Raw.init ~dst ~dst_name ~src ~src_name,dst,src)
+
+  let step (b,_,_) i = Raw.step b i
+
+  let finish (b,_,_) = Raw.finish b
+
+  let remaining (b,_,_) = Raw.remaining b
+
+  let pagecount (b,_,_) = Raw.pagecount b
 end  (* Backup *)
 
 (* Initialisation *)

[mtelvers]

Thanks @pirbo and @klakplok.

I have updated the PR with these changes.

[mmottl]

Thanks for the contribution!