Skip to content

Commit a119b79

Browse files
committed
Fix some FFI functions incorrectly being unsafe.
Those functions can be reentrant or do IO. This fixes a case I encountered on our CI machine where `unsafe` on `PQisBusy ()` resulted in GHC RTS hangs when a postgresql notice processor was set to call back into Haskell.
1 parent 7441ca8 commit a119b79

File tree

1 file changed

+80
-15
lines changed

1 file changed

+80
-15
lines changed

src/Database/PostgreSQL/LibPQ.hsc

Lines changed: 80 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2373,6 +2373,19 @@ loUnlink connection oid
23732373
negError =<< c_lo_unlink c oid
23742374

23752375

2376+
-- Reminder on `unsafe` FFI:
2377+
--
2378+
-- - `unsafe` calls MUST NOT do any IO!
2379+
-- - `unsafe` calls MUST NOT be reentrant!
2380+
-- This means it must be guaranteed that there exists no possible
2381+
-- code path that can call back into Haskell.
2382+
--
2383+
-- Here is an example of the seemingly innocent function `PQisBusy()`
2384+
-- actually being reentrant in some cases:
2385+
-- `PQisBusy()` in some cases calls `pqParseInput3()`
2386+
-- which calls `pqGetErrorNotice3()` which can call back into a
2387+
-- user-defined error notice processing callback if set,
2388+
-- which the user might have set to be a Haskell function.
23762389

23772390
foreign import ccall "libpq-fe.h PQconnectdb"
23782391
c_PQconnectdb :: CString ->IO (Ptr PGconn)
@@ -2383,51 +2396,66 @@ foreign import ccall "libpq-fe.h PQconnectStart"
23832396
foreign import ccall "libpq-fe.h PQconnectPoll"
23842397
c_PQconnectPoll :: Ptr PGconn ->IO CInt
23852398

2399+
-- | `unsafe` is OK because `PQdb` calls no functions.
23862400
foreign import ccall unsafe "libpq-fe.h PQdb"
23872401
c_PQdb :: Ptr PGconn -> IO CString
23882402

2403+
-- | `unsafe` is OK because `PQuser` calls no functions.
23892404
foreign import ccall unsafe "libpq-fe.h PQuser"
23902405
c_PQuser :: Ptr PGconn -> IO CString
23912406

2407+
-- | `unsafe` is OK because `PQpass` calls no functions.
23922408
foreign import ccall unsafe "libpq-fe.h PQpass"
23932409
c_PQpass :: Ptr PGconn -> IO CString
23942410

2411+
-- | `unsafe` is OK because `PQhost` calls no functions.
23952412
foreign import ccall unsafe "libpq-fe.h PQhost"
23962413
c_PQhost :: Ptr PGconn -> IO CString
23972414

2415+
-- | `unsafe` is OK because `PQport` calls no functions.
23982416
foreign import ccall unsafe "libpq-fe.h PQport"
23992417
c_PQport :: Ptr PGconn -> IO CString
24002418

2419+
-- | `unsafe` is OK because `PQport` calls no functions.
24012420
foreign import ccall unsafe "libpq-fe.h PQoptions"
24022421
c_PQoptions :: Ptr PGconn -> IO CString
24032422

2423+
-- | `unsafe` is OK because `PQbackendPID` calls no functions.
24042424
foreign import ccall unsafe "libpq-fe.h PQbackendPID"
24052425
c_PQbackendPID :: Ptr PGconn -> IO CInt
24062426

2427+
-- | `unsafe` is OK because `PQconnectionNeedsPassword` calls no functions
2428+
-- (it calls `PQpass`, which does the same).
24072429
foreign import ccall unsafe "libpq-fe.h PQconnectionNeedsPassword"
24082430
c_PQconnectionNeedsPassword :: Ptr PGconn -> IO CInt
24092431

2432+
-- | `unsafe` is OK because `PQconnectionUsedPassword` calls no functions.
24102433
foreign import ccall unsafe "libpq-fe.h PQconnectionUsedPassword"
24112434
c_PQconnectionUsedPassword :: Ptr PGconn -> IO CInt
24122435

2436+
-- | `unsafe` is OK because `PQstatus` calls no functions.
24132437
foreign import ccall unsafe "libpq-fe.h PQstatus"
24142438
c_PQstatus :: Ptr PGconn -> IO CInt
24152439

2440+
-- | `unsafe` is OK because `PQtransactionStatus` calls no functions.
24162441
foreign import ccall unsafe "libpq-fe.h PQtransactionStatus"
24172442
c_PQtransactionStatus :: Ptr PGconn -> IO CInt
24182443

24192444
foreign import ccall "libpq-fe.h PQparameterStatus"
24202445
c_PQparameterStatus :: Ptr PGconn -> CString -> IO CString
24212446

2447+
-- | `unsafe` is OK because `PQprotocolVersion` calls no functions.
24222448
foreign import ccall unsafe "libpq-fe.h PQprotocolVersion"
24232449
c_PQprotocolVersion :: Ptr PGconn -> IO CInt
24242450

2451+
-- | `unsafe` is OK because `PQserverVersion` calls no functions.
24252452
foreign import ccall unsafe "libpq-fe.h PQserverVersion"
24262453
c_PQserverVersion :: Ptr PGconn -> IO CInt
24272454

24282455
foreign import ccall "dynamic"
24292456
mkLibpqVersion :: FunPtr Int -> Int
24302457

2458+
-- | `unsafe` is OK because `PQsocket` calls no functions.
24312459
foreign import ccall unsafe "libpq-fe.h PQsocket"
24322460
c_PQsocket :: Ptr PGconn -> IO CInt
24332461

@@ -2451,6 +2479,7 @@ foreign import ccall "libpq-fe.h PQresetStart"
24512479
foreign import ccall "libpq-fe.h PQresetPoll"
24522480
c_PQresetPoll :: Ptr PGconn ->IO CInt
24532481

2482+
-- | `unsafe` is OK because `PQclientEncoding` calls no functions.
24542483
foreign import ccall unsafe "libpq-fe.h PQclientEncoding"
24552484
c_PQclientEncoding :: Ptr PGconn -> IO CInt
24562485

@@ -2461,6 +2490,7 @@ foreign import ccall "libpq-fe.h PQsetClientEncoding"
24612490
c_PQsetClientEncoding :: Ptr PGconn -> CString -> IO CInt
24622491

24632492
type PGVerbosity = CInt
2493+
-- | `unsafe` is OK because `PQclientEncoding` calls no functions.
24642494
foreign import ccall unsafe "libpq-fe.h PQsetErrorVerbosity"
24652495
c_PQsetErrorVerbosity :: Ptr PGconn -> PGVerbosity -> IO PGVerbosity
24662496

@@ -2507,21 +2537,23 @@ foreign import ccall "libpq-fe.h &PQfreeCancel"
25072537
foreign import ccall "libpq-fe.h PQcancel"
25082538
c_PQcancel :: Ptr PGcancel -> CString -> CInt -> IO CInt
25092539

2510-
foreign import ccall unsafe "libpq-fe.h PQnotifies"
2540+
foreign import ccall "libpq-fe.h PQnotifies"
25112541
c_PQnotifies :: Ptr PGconn -> IO (Ptr Notify)
25122542

25132543
foreign import ccall "libpq-fe.h PQconsumeInput"
25142544
c_PQconsumeInput :: Ptr PGconn -> IO CInt
25152545

2516-
foreign import ccall unsafe "libpq-fe.h PQisBusy"
2546+
foreign import ccall "libpq-fe.h PQisBusy"
25172547
c_PQisBusy :: Ptr PGconn -> IO CInt
25182548

25192549
foreign import ccall "libpq-fe.h PQsetnonblocking"
25202550
c_PQsetnonblocking :: Ptr PGconn -> CInt -> IO CInt
25212551

2552+
-- | `unsafe` is OK because `PQisnonblocking` calls only a macro that calls no functions.
25222553
foreign import ccall unsafe "libpq-fe.h PQisnonblocking"
25232554
c_PQisnonblocking :: Ptr PGconn -> IO CInt
25242555

2556+
-- | `unsafe` is OK because `PQsetSingleRowMode` calls no functions.
25252557
foreign import ccall unsafe "libpq-fe.h PQsetSingleRowMode"
25262558
c_PQsetSingleRowMode :: Ptr PGconn -> IO CInt
25272559

@@ -2553,67 +2585,91 @@ foreign import ccall "libpq-fe.h PQdescribePortal"
25532585
foreign import ccall "libpq-fe.h &PQclear"
25542586
p_PQclear :: FunPtr (Ptr PGresult ->IO ())
25552587

2588+
-- | `unsafe` is OK because `PQresultStatus` calls no functions.
25562589
foreign import ccall unsafe "libpq-fe.h PQresultStatus"
25572590
c_PQresultStatus :: Ptr PGresult -> IO CInt
25582591

2559-
foreign import ccall unsafe "libpq-fe.h PQresStatus"
2592+
-- Must not be `unsafe` because `PQresStatus` may use `libpq_gettext()` that may do IO.
2593+
foreign import ccall "libpq-fe.h PQresStatus"
25602594
c_PQresStatus :: CInt -> IO CString
25612595

2596+
-- | `unsafe` is OK because `PQresultStatus` calls no functions.
25622597
foreign import ccall unsafe "libpq-fe.h PQresultErrorMessage"
25632598
c_PQresultErrorMessage :: Ptr PGresult -> IO CString
25642599

25652600
foreign import ccall "libpq-fe.h PQresultErrorField"
25662601
c_PQresultErrorField :: Ptr PGresult -> CInt -> IO CString
25672602

2603+
-- | `unsafe` is OK because `PQntuples` calls no functions.
25682604
foreign import ccall unsafe "libpq-fe.h PQntuples"
25692605
c_PQntuples :: Ptr PGresult -> CInt
25702606

2607+
-- | `unsafe` is OK because `PQnfields` calls no functions.
25712608
foreign import ccall unsafe "libpq-fe.h PQnfields"
25722609
c_PQnfields :: Ptr PGresult -> CInt
25732610

2574-
foreign import ccall unsafe "libpq-fe.h PQfname"
2611+
-- Must not be `unsafe` because `PQfname` may call `check_field_number()` which may call `pqInternalNotice()`.
2612+
foreign import ccall "libpq-fe.h PQfname"
25752613
c_PQfname :: Ptr PGresult -> CInt -> IO CString
25762614

2615+
-- | `unsafe` is OK because `PQfnumber` calls only `pg_tolower()`, which calls
2616+
-- only C stdlib functions that cannot be reentrant.
2617+
-- Note `pg_tolower()` also calls `strdup()` and `free()`, so if one of those
2618+
-- was overridden with e.g. glibc hooks to call back into Haskell, this would be wrong.
2619+
-- However, these functions are generally considered to not be reentrant.
25772620
foreign import ccall unsafe "libpq-fe.h PQfnumber"
25782621
c_PQfnumber :: Ptr PGresult -> CString -> IO CInt
25792622

2580-
foreign import ccall unsafe "libpq-fe.h PQftable"
2623+
-- Must not be `unsafe` because `PQftable` may call `check_field_number()` which may call `pqInternalNotice()`.
2624+
foreign import ccall "libpq-fe.h PQftable"
25812625
c_PQftable :: Ptr PGresult -> CInt -> IO Oid
25822626

2583-
foreign import ccall unsafe "libpq-fe.h PQftablecol"
2627+
-- Must not be `unsafe` because `PQftablecol` may call `check_field_number()` which may call `pqInternalNotice()`.
2628+
foreign import ccall "libpq-fe.h PQftablecol"
25842629
c_PQftablecol :: Ptr PGresult -> CInt -> IO CInt
25852630

2586-
foreign import ccall unsafe "libpq-fe.h PQfformat"
2631+
-- Must not be `unsafe` because `PQfformat` may call `check_field_number()` which may call `pqInternalNotice()`.
2632+
foreign import ccall "libpq-fe.h PQfformat"
25872633
c_PQfformat :: Ptr PGresult -> CInt -> IO CInt
25882634

2635+
-- Must not be `unsafe` because `PQftype` may call `check_field_number()` which may call `pqInternalNotice()`.
25892636
foreign import ccall unsafe "libpq-fe.h PQftype"
25902637
c_PQftype :: Ptr PGresult -> CInt -> IO Oid
25912638

2592-
foreign import ccall unsafe "libpq-fe.h PQfmod"
2639+
-- Must not be `unsafe` because `PQfmod` may call `check_field_number()` which may call `pqInternalNotice()`.
2640+
foreign import ccall "libpq-fe.h PQfmod"
25932641
c_PQfmod :: Ptr PGresult -> CInt -> IO CInt
25942642

2595-
foreign import ccall unsafe "libpq-fe.h PQfsize"
2643+
-- Must not be `unsafe` because `PQfsize` may call `check_field_number()` which may call `pqInternalNotice()`.
2644+
foreign import ccall "libpq-fe.h PQfsize"
25962645
c_PQfsize :: Ptr PGresult -> CInt -> IO CInt
25972646

2598-
foreign import ccall unsafe "libpq-fe.h PQgetvalue"
2647+
-- Must not be `unsafe` because `PQgetvalue` may call `check_tuple_field_number()` which may call `pqInternalNotice()`.
2648+
foreign import ccall "libpq-fe.h PQgetvalue"
25992649
c_PQgetvalue :: Ptr PGresult -> CInt -> CInt -> IO CString
26002650

2601-
foreign import ccall unsafe "libpq-fe.h PQgetisnull"
2651+
-- Must not be `unsafe` because `PQgetisnull` may call `check_tuple_field_number()` which may call `pqInternalNotice()`.
2652+
foreign import ccall "libpq-fe.h PQgetisnull"
26022653
c_PQgetisnull :: Ptr PGresult -> CInt -> CInt -> IO CInt
26032654

2604-
foreign import ccall unsafe "libpq-fe.h PQgetlength"
2655+
-- Must not be `unsafe` because `PQgetlength` may call `check_tuple_field_number()` which may call `pqInternalNotice()`.
2656+
foreign import ccall "libpq-fe.h PQgetlength"
26052657
c_PQgetlength :: Ptr PGresult -> CInt -> CInt -> IO CInt
26062658

2659+
-- | `unsafe` is OK because `PQnparams` calls no functions.
26072660
foreign import ccall unsafe "libpq-fe.h PQnparams"
26082661
c_PQnparams :: Ptr PGresult -> IO CInt
26092662

2610-
foreign import ccall unsafe "libpq-fe.h PQparamtype"
2663+
-- Must not be `unsafe` because `PQparamtype` may call `check_param_number()` which may call `pqInternalNotice()`.
2664+
foreign import ccall "libpq-fe.h PQparamtype"
26112665
c_PQparamtype :: Ptr PGresult -> CInt -> IO Oid
26122666

2667+
-- | `unsafe` is OK because `PQcmdStatus` calls no functions.
26132668
foreign import ccall unsafe "libpq-fe.h PQcmdStatus"
26142669
c_PQcmdStatus :: Ptr PGresult -> IO CString
26152670

2616-
foreign import ccall unsafe "libpq-fe.h PQcmdTuples"
2671+
-- Must not be `unsafe` because `PQcmdTuples` may call `pqInternalNotice()`.
2672+
foreign import ccall "libpq-fe.h PQcmdTuples"
26172673
c_PQcmdTuples :: Ptr PGresult -> IO CString
26182674

26192675
foreign import ccall "libpq-fe.h PQescapeStringConn"
@@ -2636,33 +2692,42 @@ foreign import ccall "libpq-fe.h PQunescapeBytea"
26362692
-> Ptr CSize
26372693
-> IO (Ptr Word8) -- Actually (IO (Ptr CUChar))
26382694

2639-
foreign import ccall unsafe "libpq-fe.h PQescapeIdentifier"
2695+
-- Must not be `unsafe` because `PQescapeIdentifier` may call `libpq_gettext()` that may do IO.
2696+
foreign import ccall "libpq-fe.h PQescapeIdentifier"
26402697
c_PQescapeIdentifier :: Ptr PGconn
26412698
-> CString
26422699
-> CSize
26432700
-> IO CString
26442701

2702+
-- | `unsafe` is OK because `PQfreemem` only calls `free()`.
26452703
foreign import ccall unsafe "libpq-fe.h &PQfreemem"
26462704
p_PQfreemem :: FunPtr (Ptr a -> IO ())
26472705

2706+
-- | `unsafe` is OK because `PQfreemem` only calls `free()`.
26482707
foreign import ccall unsafe "libpq-fe.h PQfreemem"
26492708
c_PQfreemem :: Ptr a -> IO ()
26502709

2710+
-- | `unsafe` is OK because `hs_postgresql_libpq_malloc_noticebuffer` only calls `malloc()`.
26512711
foreign import ccall unsafe "noticehandlers.h hs_postgresql_libpq_malloc_noticebuffer"
26522712
c_malloc_noticebuffer :: IO (Ptr CNoticeBuffer)
26532713

2714+
-- | `unsafe` is OK because `hs_postgresql_libpq_malloc_noticebuffer` only calls `free()`.
26542715
foreign import ccall unsafe "noticehandlers.h hs_postgresql_libpq_free_noticebuffer"
26552716
c_free_noticebuffer :: Ptr CNoticeBuffer -> IO ()
26562717

2718+
-- | `unsafe` is OK because `hs_postgresql_libpq_get_notice` calls no functions.
26572719
foreign import ccall unsafe "noticehandlers.h hs_postgresql_libpq_get_notice"
26582720
c_get_notice :: Ptr CNoticeBuffer -> IO (Ptr PGnotice)
26592721

2722+
-- | `unsafe` is OK because `hs_postgresql_libpq_discard_notices` calls no functions.
26602723
foreign import ccall unsafe "noticehandlers.h &hs_postgresql_libpq_discard_notices"
26612724
p_discard_notices :: FunPtr NoticeReceiver
26622725

2726+
-- | `unsafe` is OK because `hs_postgresql_libpq_store_notices` calls only `PQresultErrorMessage`, which calls no functions.
26632727
foreign import ccall unsafe "noticehandlers.h &hs_postgresql_libpq_store_notices"
26642728
p_store_notices :: FunPtr NoticeReceiver
26652729

2730+
-- | `unsafe` is OK because `PQsetNoticeReceiver` calls no functions.
26662731
foreign import ccall unsafe "libpq-fe.h PQsetNoticeReceiver"
26672732
c_PQsetNoticeReceiver :: Ptr PGconn -> FunPtr NoticeReceiver -> Ptr CNoticeBuffer -> IO (FunPtr NoticeReceiver)
26682733

0 commit comments

Comments
 (0)