search

haskell / unix

POSIX functionality
https://hackage.haskell.org/package/unix
Other
107 stars 92 forks source link

waitToSetLock can block but is an unsafe FFI import #310

Open hsyl20 opened 10 months ago

hsyl20 commented 10 months ago

waitToSetLock uses c_fcntl_lock which is an unsafe FFI import, but the call may block indefinitely.

It was the root cause of https://gitlab.haskell.org/ghc/ghc/-/issues/15485 in GHC <= 9.0

This is related to #34 but this particular call should be uncontroversial to fix (I think).

hasufell commented 10 months ago

@hsyl20 I'm not entirely sure what you're proposing. Can you open a PR?

hsyl20 commented 10 months ago

c_fcntl_lock is defined like this in base:System.Posix.Internals (for the non JS variant):

foreign import capi unsafe "HsBase.h fcntl"
   c_fcntl_lock  :: CInt -> CInt -> Ptr CFLock -> IO CInt

It's an unsafe foreign import; we shouldn't use it with FSETLKW which blocks indefinitely. We should define and use a safe foreign import instead (e.g. c_fcntl_lock_safe).

I don't know why c_fcntl_lock is exposed from base but it makes opening a PR more difficult. Either we add the safe foreign import to base but it probably require a CLC proposal, or we add it to unix but then it's weird to have one part here and the other in base, or we move everything to unix but it probably requires a CLC proposal and some deprecation period too.

hasufell commented 10 months ago

Any opinions @Bodigrim @hs-viktor ?

hsyl20 commented 10 months ago

Another alternative: add a safety parameter to c_fcntl_lock in base to force the caller to explicitly select between the safe/unsafe versions of the call to use. We'd just need some CPP in unix to adapt to the new API.

vdukhovni commented 10 months ago

We should stop using the import from System.Posix.Internals and use a private safe foreign import.

vdukhovni commented 10 months ago

No CLC proposal required, a private foreign import in System/Posix/IO/Common.hsc does not change any APIs. What the CLC decides to do with the existing public API in System.Posix.Internals is a separate issue, not material to unix.