L.hGetContents should not close handle

[yaitskov]

Data.ByteString.Lazy.hGetContents closes handle. This is not obvious and redundant, because handle is usually created in a bracket which takes care of closing handle.

hGetContents should be deprecated and
hGetContents' introduced with new semantics.

data GitObject
  = GitObject
    { gobHash :: !(Digest SHA1State)
    , gobOrigin :: !FilePath
    }
    deriving (Show, Eq)

mkGitObject :: PhoenixM m => FilePath -> m (Maybe GitObject)
mkGitObject fp = do
  s <- asks inHandlesSem
  U.bracket_ (U.waitQSem s) (U.signalQSem s) $
    U.withBinaryFile fp U.ReadMode $ \inH -> do
      magicBs <- hGet inH 2
      if zlibP magicBs
        then do
          !headerBs <- (magicBs <>) <$> hGet inH 10
          (`U.catch` skipCorruptedFile inH) $ do
            if gitObjectP $ Z.decompress (toLazy headerBs)
              then do
                !goh <- sha1 . Z.decompress . (toLazy headerBs <>) <$> hGetContents inH
                pure . Just $ GitObject goh fp
              else pure Nothing
        else pure Nothing
  where
    skipCorruptedFile inH (e :: Z.DecompressError) =
      case e of
        Z.TruncatedInput -> do
          fsz <- liftIO $ U.hFileSize inH

[yaitskov]

haskell/bytestring#707

[thielema]

On Sun, 29 Jun 2025, Daniil Iaitskov wrote: Data.ByteString.Lazy.hGetContents closes handle. This is not obvious and redundant, because handle is usually created in a bracket which takes care of closing handle.

You must not use hGetContents with 'bracket', because you cannot easily control when the last byte is read from the file via lazy evaluation.

[yaitskov]

You must not use hGetContents with 'bracket', because you cannot easily control when the last byte is read from the file via lazy evaluation.

For this particular case:
I need version of hGetContents which doesn't close handle. L.readFile is not applicable because some files are not fully read (because zip file might have trash tail) and it causes too many open files exception!

I understand and accept potential problems with lazy evaluation, but it doesn't justify breaking responsibility convention.

[TeofilC]

Quoting from the readme:

Maintainers of Core Libraries may at their own accord seek CLC approval for controversial changes, but are not required to do so.

So until a bytestring maintainer asks for a CLC proposal there is no need to open one

[yaitskov]

This function semantic is against any abc design book.

[yaitskov]

data Scope = Free | Bracked deriving (Eq, Show)

newtype BsHandle (s :: Scope) = BsHandle Handle

class GetContents a where
  hGetContents :: BsHandle a -> IO ByteString

instance GetContents Free where
  hGetContents (BsHandle h) = Origin.hGetContents h

instance GetContents Bracked where
  hGetContents (BsHandle h) = hGetContentsOnly h
  
openFile :: FilePath -> IOMode -> IO BsHandle Free

hClose ::  IO BsHandle Free -> IO ()

withBinaryFile :: FilePath -> IOMode -> (BsHandle Bracked -> IO r) -> IO r

[yaitskov]

haskell/bytestring#708

[tomjaguarpaw]

Lazy ByteString (and Text) are a misdesign. If we had had robust streaming implementations before they were introduced then we would never have felt the need for them. I suggest instead using an effect system that supports streams and prompt finalization, for example Bluefin.

[hasufell]

I think lazy bytestring is fine. The issue is with lazy IO.

[tomjaguarpaw]

The issue is with lazy IO.

That's true. And there is technically some purpose to lazy bytestring in the absence of lazy IO, that is, when you want to produce and consume large bytestrings in pure code, and processing individual chunks in a list is inappropriate for some reason. I think such use cases are pretty niche.

[hasufell]

The issue is with lazy IO.

That's true. And there is technically some purpose to lazy bytestring in the absence of lazy IO, that is, when you want to produce and consume large bytestrings in pure code, and processing individual chunks in a list is inappropriate for some reason. I think such use cases are pretty niche.

I don't think it's that uncommon for bytestring. As the documentation for Data.ByteString.Builder indicates, it can be useful to turn data into an encoded bytestring.

[tomjaguarpaw]

Indeed, it's useful to be able to produce, process and consume purely. What is less useful (though not useless), is to produce, process and consume only purely. For example, one might produce a lazy bytestring by serializing a pure value, process it purely (by way of example: to remove whitespace) and then consume it by writing it to disk or sending it over the network. The Data.ByteString.Builder documentation anticipates exactly this:

LazyByteStrings can be efficiently consumed by functions that write them to a file or send them over a network socket.

It's much less common to also consume it purely, if you already produced and processed it purely. I'm not saying it never happens. I'm saying it's much less common.

Now, this is somewhat tangential to the main point, which is that even if we sometimes want to use lazy bytestrings completely purely, we often want to use them with IO, which, given the design, will have to be lazy IO. There lie an awful lot of dangers. Oh dear.

So, I still believe in my original point: if we had had robust streaming implementations before they were introduced then we would never have felt the need for lazy text and bytestring. That doesn't mean they're useless as they are, it just means a better design subsumes them. So, I recommend to anyone trying to do what OP is doing to, instead of lazy bytestring (or text), use an effect system with support for streams and prompt finalization.

[hasufell]

Great discussion, but I'll close this for now, unless bytestring maintainers want our input.