haskellari / postgresql-libpq

Low-level Haskell bindings for libpq
BSD 3-Clause "New" or "Revised" License
20 stars 25 forks source link

Make `finish` wait until the connection is actually finished. #20

Closed nh2 closed 2 years ago

nh2 commented 2 years ago

Until now, finish only scheduled the connection to be closed, resulting in connection existing even after finish (and thus, after higher-level functions like postgresql-simple's close).

Beyond incontrollable resource usage, this could also lead to segfaults if the user employed any libpq functionality that uses Haskell callbacks, for example "Notice Processing" (https://www.postgresql.org/docs/14/libpq-notice-processing.html).

Giving a Haskell callback to libpq usually involves code of shape:

fp <- createFunctionPointer ... -- function that accepts a PGConn
-- pass fp to libpq
(LibPQ.finish connection) `finally` (freeHaskellFunPtr fp)

It is critical that the function pointer fp and related resources get freed only after they are last called by libpq.

But the delayed nature of finish so far did not provide this guarantee: The (LibPQ.finish connection)finally(freeHaskellFunPtr fp) from the example wouldn't actually ensure that finish "finishes" anything before the finally and the freeing happen, thus leading to segfaults when libpq called into already-freed function pointers.

This commit fixes it by making finish block on an MVar that is put only after PQFinish was actually called.

nh2 commented 2 years ago

Test which would fail before this change.

Having tried for a while, this seems to be difficult to do.

The docs on Foreign.Concurrent.newForeignPtr say:

The storage manager will start the finalizer, in a separate thread, some time after the last reference to the ForeignPtr is dropped. There is no guarantee of promptness, and in fact there is no guarantee that the finalizer will eventually run at all.

But so far I have not managed to find a way to explicitly provoke this behaviour in a contained example.

For starters, I do not know how to reliably trigger the

The storage manager will start the finalizer, in a separate thread

behaviour. Consider:

module Main (main) where

import           Control.Concurrent (myThreadId)
import           Foreign.Marshal.Alloc (mallocBytes)
import           Foreign.ForeignPtr (finalizeForeignPtr)
import qualified Foreign.Concurrent as FC

main :: IO ()
main = do
    let finalizerAction :: IO ()
        finalizerAction = do
            i <- myThreadId
            putStrLn $ "Finalizer running in " ++ show i

    ptr <- mallocBytes 10
    fp <- FC.newForeignPtr ptr finalizerAction

    i <- myThreadId
    putStrLn $ "Calling finalizeForeignPtr in " ++ show i

    finalizeForeignPtr fp

Running this on GHC 8.10.7 on Linux linked with -threaded gives:

Calling finalizeForeignPtr in ThreadId 4
Finalizer running in ThreadId 4

Here it's running the finalizer in the same Haskell thread. As per docs, this isn't guaranteed, but I'm not sure how to force it to not happen.


In any case, PQfinish() guarantees that afer it returns successfully, the connection is closed.

I think it would make sense for the Haskell finish to guarantee the same.