paurkedal / ocaml-caqti

Cooperative-threaded access to relational data
https://paurkedal.github.io/ocaml-caqti/index.html
GNU Lesser General Public License v3.0
307 stars 36 forks source link

"Invalid concurrent usage of PostgreSQL connection detected" when raising an exception in with_connection callback #116

Closed mooreryan closed 6 months ago

mooreryan commented 7 months ago

I was playing around with the with_connection function trying to break it by doing strange things. For example not binding on the result of executing a query:

module Caqti_open = struct
  include Caqti_type.Std
  include Caqti_request.Infix
end

let check =
  let request = Caqti_open.((unit ->! int) "SELECT 1") in
  fun (module Db : Caqti_async.CONNECTION) -> Db.find request ()

let _ =
  Thread_safe.block_on_async_exn (fun () ->
      Caqti_async.with_connection db_connection_uri (fun db ->
          (* Should bind here but I'm not for whatever reason... *)
          let _x = check db in
          failwith "oh no" ) )

Gives the monitor error:

  (monitor.ml.Error
   (Failure "Invalid concurrent usage of PostgreSQL connection detected.")
   ("Raised at Stdlib.failwith in file \"stdlib.ml\", line 29, characters 17-33"
    "Called from Caqti_driver_postgresql.Connect_functor.Make_connection_base.using_db in file \"caqti-driver-postgresql/lib/caqti_driver_postgresql.ml\", line 850, characters 8-78"
    "Called from Caqti_platform__Switch.Make.release.loop in file \"caqti/lib-platform/switch.ml\", line 68, characters 8-34"
    "Called from Caqti_async.Fiber.finally.(fun) in file \"caqti-async/lib/caqti_async.ml\", line 54, characters 20-24"
    "Called from Async_kernel__Deferred0.bind.(fun) in file \"src/deferred0.ml\", line 54, characters 64-69"
    "Called from Async_kernel__Job_queue.run_jobs in file \"src/job_queue.ml\", line 180, characters 6-47"
    "Caught by monitor block_on_async"))

rather than the "oh no" exception as I might expect. This also occurs with the caqti-lwt, however not with the blocking version of caqti.

I'm guessing that this is a case of "don't do something like this inside the with_connection function...but maybe it is a bug?

There are a couple of issues that are potentially related:

mooreryan commented 7 months ago

Actually I think this may be related: https://github.com/paurkedal/ocaml-caqti/issues/12#issuecomment-380820885 (and the couple of comments surrounding it....

mooreryan commented 7 months ago

I forgot to mention that returning an error rather than raising will also trigger this:

let _ =
  Thread_safe.block_on_async_exn (fun () ->
      Caqti_async.with_connection db_connection_uri (fun db ->
          let _x = check db in
          return (Error (`Msg "oops")) ) )
paurkedal commented 7 months ago

The missing bind, as you note in the comment, is the problem. The database connection is only valid inside the with_connection callback. What can happen here is that the check db call gets scheduled and can then be schedule at a future point. My guess is that it get scheduled while with_connection is waiting for an internal call to disconnect to complete.

paurkedal commented 6 months ago

I'm closing this as it's not a bug, but a limitation of the type-safety provided by the with_connection (in lack of linear types).