haskell-grpc-native / http2-client

A native HTTP2 client in Haskell
BSD 3-Clause "New" or "Revised" License
43 stars 27 forks source link

Dodgy exception handling in `runHttp2Client` #86

Open parsonsmatt opened 1 year ago

parsonsmatt commented 1 year ago

It's possible for the _close to never be called in the event of an exception. These lines of code are especially problematic:

runHttp2Client conn encoderBufSize decoderBufSize initSettings goAwayHandler fallbackHandler mainHandler = do
    (incomingLoop, initClient) <- initHttp2Client conn encoderBufSize decoderBufSize goAwayHandler fallbackHandler
    withAsync incomingLoop $ \aIncoming -> do
        settsIO <- _initSettings initClient initSettings
        withAsync settsIO $ \aSettings -> do
            let client = Http2Client {
              _settings            = _initSettings initClient
            , _ping                = _initPing initClient
            , _goaway              = _initGoaway initClient
            , _close               =
                _initStop initClient >> _initClose initClient
            , _startStream         = _initStartStream initClient
            , _incomingFlowControl = _initIncomingFlowControl initClient
            , _outgoingFlowControl = _initOutgoingFlowControl initClient
            , _payloadSplitter     = _initPaylodSplitter initClient
            , _asyncs              = Http2ClientAsyncs aSettings aIncoming
            }
            linkAsyncs client
            ret <- mainHandler client
            _close client
            return ret

Fortunately the fix should be relatively straightforward - doing mainHandler client `finally` _close client should fix that _close won't be run if mainHandler throws an exception.

ProofOfKeags commented 1 year ago

This explains something I've run into I think. Thanks for catching it. If you want to do a PR I can merge it ASAP. Otherwise I'll see if I can get to it tonight. Haven't worked on this for a bit now so need to get it set up otherwise I'd do it now.

lucasdicioccio commented 1 year ago

oh good catch! your patch proposition should work

I'll try something over a lunch next week.