[sqlite] reset prepared statement eagerly

Summary:
This makes sure that sqlite doesn't hold read locks for longer than necessary,
which could starve the process of cleaning up the WAL file. This ensures that
the statement is reset as soon as we're done reading.

I haven't observed a difference with this change, and could not find evidence
that it should change something in the docs. Internet wisdom pointed at this as
a potential issue and I was observing it in another change, so it's good to
rule it out.

Reviewed By: mbouaziz

Differential Revision: D6404353

fbshipit-source-id: a123cd6

infer/src/IR/Attributes.ml

@@ -23,7 +23,7 @@ let proc_kind_of_attr (proc_attributes: ProcAttributes.t) =
   else ProcUndefined
 
 
-let get_replace_statement =
+let replace_statement =
   (* The innermost SELECT returns either the current attributes_kind and source_file associated with
      the given proc name, or default values of (-1,""). These default values have the property that
      they are always "less than" any legit value. More precisely, MAX ensures that some value is
@@ -56,19 +56,19 @@ FROM (
 
 
 let replace pname_blob akind loc_file attr_blob =
-  let replace_stmt = get_replace_statement () in
-  Sqlite3.bind replace_stmt 1 (* :pname *) pname_blob
-  |> SqliteUtils.check_sqlite_error ~log:"replace bind pname" ;
-  Sqlite3.bind replace_stmt 2 (* :akind *) (Sqlite3.Data.INT (int64_of_attributes_kind akind))
-  |> SqliteUtils.check_sqlite_error ~log:"replace bind attribute kind" ;
-  Sqlite3.bind replace_stmt 3 (* :sfile *) loc_file
-  |> SqliteUtils.check_sqlite_error ~log:"replace bind source file" ;
-  Sqlite3.bind replace_stmt 4 (* :pattr *) attr_blob
-  |> SqliteUtils.check_sqlite_error ~log:"replace bind proc attributes" ;
-  SqliteUtils.sqlite_unit_step ~finalize:false ~log:"Attributes.replace" replace_stmt
-
-
-let get_find_more_defined_statement =
+  ResultsDatabase.with_registered_statement replace_statement ~f:(fun replace_stmt ->
+      Sqlite3.bind replace_stmt 1 (* :pname *) pname_blob
+      |> SqliteUtils.check_sqlite_error ~log:"replace bind pname" ;
+      Sqlite3.bind replace_stmt 2 (* :akind *) (Sqlite3.Data.INT (int64_of_attributes_kind akind))
+      |> SqliteUtils.check_sqlite_error ~log:"replace bind attribute kind" ;
+      Sqlite3.bind replace_stmt 3 (* :sfile *) loc_file
+      |> SqliteUtils.check_sqlite_error ~log:"replace bind source file" ;
+      Sqlite3.bind replace_stmt 4 (* :pattr *) attr_blob
+      |> SqliteUtils.check_sqlite_error ~log:"replace bind proc attributes" ;
+      SqliteUtils.sqlite_unit_step ~finalize:false ~log:"Attributes.replace" replace_stmt )
+
+
+let find_more_defined_statement =
   ResultsDatabase.register_statement
     {|
 SELECT attr_kind
@@ -79,31 +79,33 @@ WHERE proc_name = :pname
 
 
 let should_try_to_update pname_blob akind =
-  let find_stmt = get_find_more_defined_statement () in
-  Sqlite3.bind find_stmt 1 (* :pname *) pname_blob
-  |> SqliteUtils.check_sqlite_error ~log:"replace bind pname" ;
-  Sqlite3.bind find_stmt 2 (* :akind *) (Sqlite3.Data.INT (int64_of_attributes_kind akind))
-  |> SqliteUtils.check_sqlite_error ~log:"replace bind attribute kind" ;
-  SqliteUtils.sqlite_result_step ~finalize:false ~log:"Attributes.replace" find_stmt
-  |> (* there is no entry with a strictly larger "definedness" for that proc name *) Option.is_none
+  ResultsDatabase.with_registered_statement find_more_defined_statement ~f:(fun find_stmt ->
+      Sqlite3.bind find_stmt 1 (* :pname *) pname_blob
+      |> SqliteUtils.check_sqlite_error ~log:"replace bind pname" ;
+      Sqlite3.bind find_stmt 2 (* :akind *) (Sqlite3.Data.INT (int64_of_attributes_kind akind))
+      |> SqliteUtils.check_sqlite_error ~log:"replace bind attribute kind" ;
+      SqliteUtils.sqlite_result_step ~finalize:false ~log:"Attributes.replace" find_stmt
+      |> (* there is no entry with a strictly larger "definedness" for that proc name *)
+         Option.is_none )
 
 
-let get_select_statement =
+let select_statement =
   ResultsDatabase.register_statement "SELECT proc_attributes FROM attributes WHERE proc_name = :k"
 
 
-let get_select_defined_statement =
+let select_defined_statement =
   ResultsDatabase.register_statement
     "SELECT proc_attributes FROM attributes WHERE proc_name = :k AND attr_kind = %Ld"
     (int64_of_attributes_kind ProcDefined)
 
 
 let find ~defined pname_blob =
-  let select_stmt = if defined then get_select_defined_statement () else get_select_statement () in
-  Sqlite3.bind select_stmt 1 pname_blob
-  |> SqliteUtils.check_sqlite_error ~log:"find bind proc name" ;
-  SqliteUtils.sqlite_result_step ~finalize:false ~log:"Attributes.find" select_stmt
-  |> Option.map ~f:ProcAttributes.SQLite.deserialize
+  (if defined then select_defined_statement else select_statement)
+  |> ResultsDatabase.with_registered_statement ~f:(fun select_stmt ->
+         Sqlite3.bind select_stmt 1 pname_blob
+         |> SqliteUtils.check_sqlite_error ~log:"find bind proc name" ;
+         SqliteUtils.sqlite_result_step ~finalize:false ~log:"Attributes.find" select_stmt
+         |> Option.map ~f:ProcAttributes.SQLite.deserialize )
 
 
 let load pname = Typ.Procname.SQLite.serialize pname |> find ~defined:false

infer/src/IR/Cfg.ml

@@ -91,7 +91,7 @@ let check_cfg_connectedness cfg =
   iter_proc_desc cfg do_pdesc
 
 
-let get_load_statement =
+let load_statement =
   ResultsDatabase.register_statement "SELECT cfgs FROM cfg WHERE source_file = :k"
 
 
@@ -100,11 +100,11 @@ module SQLite = SqliteUtils.MarshalledData (struct
 end)
 
 let load source =
-  let load_stmt = get_load_statement () in
-  SourceFile.SQLite.serialize source |> Sqlite3.bind load_stmt 1
-  |> SqliteUtils.check_sqlite_error ~log:"load bind source file" ;
-  SqliteUtils.sqlite_result_step ~finalize:false ~log:"Cfg.load" load_stmt
-  |> Option.map ~f:SQLite.deserialize
+  ResultsDatabase.with_registered_statement load_statement ~f:(fun load_stmt ->
+      SourceFile.SQLite.serialize source |> Sqlite3.bind load_stmt 1
+      |> SqliteUtils.check_sqlite_error ~log:"load bind source file" ;
+      SqliteUtils.sqlite_result_step ~finalize:false ~log:"Cfg.load" load_stmt
+      |> Option.map ~f:SQLite.deserialize )
 
 
 (** Save the .attr files for the procedures in the cfg. *)
@@ -276,7 +276,7 @@ let mark_unchanged_pdescs cfg_new cfg_old =
   Typ.Procname.Hash.iter mark_pdesc_if_unchanged cfg_new
 
 
-let get_store_statement =
+let store_statement =
   ResultsDatabase.register_statement "INSERT OR REPLACE INTO cfg VALUES (:source, :cfgs)"
 
 
@@ -288,14 +288,14 @@ let store source_file cfg =
      OndemandCapture module relies on it - it uses existance of the cfg as a barrier to make
      sure that all attributes were written to disk (but not necessarily flushed) *)
   save_attributes source_file cfg ;
-  let store_stmt = get_store_statement () in
-  SourceFile.SQLite.serialize source_file |> Sqlite3.bind store_stmt 1
-  (* :source *)
-  |> SqliteUtils.check_sqlite_error ~log:"store bind source file" ;
-  SQLite.serialize cfg |> Sqlite3.bind store_stmt 2
-  (* :cfg *)
-  |> SqliteUtils.check_sqlite_error ~log:"store bind cfg" ;
-  SqliteUtils.sqlite_unit_step ~finalize:false ~log:"Cfg.store" store_stmt
+  ResultsDatabase.with_registered_statement store_statement ~f:(fun store_stmt ->
+      SourceFile.SQLite.serialize source_file |> Sqlite3.bind store_stmt 1
+      (* :source *)
+      |> SqliteUtils.check_sqlite_error ~log:"store bind source file" ;
+      SQLite.serialize cfg |> Sqlite3.bind store_stmt 2
+      (* :cfg *)
+      |> SqliteUtils.check_sqlite_error ~log:"store bind cfg" ;
+      SqliteUtils.sqlite_unit_step ~finalize:false ~log:"Cfg.store" store_stmt )
 
 
 (** Applies convert_instr_list to all the instructions in all the nodes of the cfg *)

infer/src/base/ResultsDatabase.ml

@@ -81,7 +81,9 @@ let db_canonicalize () =
   SqliteUtils.exec db ~log:"running VACUUM" ~stmt:"VACUUM"
 
 
-let register_statement stmt_fmt =
+type registered_stmt = unit -> Sqlite3.stmt
+
+let register_statement =
   let k stmt0 =
     let stmt_ref = ref None in
     let new_statement db =
@@ -99,12 +101,18 @@ let register_statement stmt_fmt =
       | None ->
           L.(die InternalError) "database not initialized"
       | Some stmt ->
-          Sqlite3.reset stmt |> SqliteUtils.check_sqlite_error ~log:"reset prepared statement" ;
           Sqlite3.clear_bindings stmt
           |> SqliteUtils.check_sqlite_error ~log:"clear bindings of prepared statement" ;
           stmt
   in
-  Printf.ksprintf k stmt_fmt
+  fun stmt_fmt -> Printf.ksprintf k stmt_fmt
+
+
+let with_registered_statement get_stmt ~f =
+  let stmt = get_stmt () in
+  let result = f stmt in
+  Sqlite3.reset stmt |> SqliteUtils.check_sqlite_error ~log:"reset prepared statement" ;
+  result
 
 
 let do_db_close db =

infer/src/base/ResultsDatabase.mli

@@ -31,7 +31,9 @@ val db_close : unit -> unit
 val create_db : unit -> unit
 (** create the database file and initialize all the necessary tables *)
 
-val register_statement : ('a, unit, string, unit -> Sqlite3.stmt) Base.format4 -> 'a
+type registered_stmt
+
+val register_statement : ('a, unit, string, registered_stmt) Base.format4 -> 'a
 (** Return a function unit -> Sqlite3.stmt that can be called (once the DB has been initialized) to
     get the prepared statement corresponding to the current DB connection. Use this to prepare
     statements only once per DB connection.
@@ -39,3 +41,5 @@ val register_statement : ('a, unit, string, unit -> Sqlite3.stmt) Base.format4 -
     In particular, clients of this need not worry about calling [Sqlite3.finalize] on the returned
     statement, or about generating new statements when the connection to the DB changes: this is all
     handled internally. *)
+
+val with_registered_statement : registered_stmt -> f:(Sqlite3.stmt -> 'a) -> 'a