Potential Resource leak in KeyedMap

[lpsmith]

https://github.com/snoyberg/http-client/blob/a9a1a1f76c44127d67695d90d5abdbf585052c82/http-client/Data/KeyedPool.hs#L165

While I was chasing down GHC bug #17439, I ended up reviewing some parts of http-client code. It looks like the mask_ is misplaced to me. We want to ensure that destroy is called on everything we remove from the STM transaction, and it looks like there's a theoretical opportunity for an async exception occurs after the STM transaction commits but before we reach the mask_. (Nice use of join . atomically, by the way!)

I proposed a nice solution to the haskell-libraries mailing list here; but in the meantime, you could either use the naive implementation of joinAtomicallyWithMask (you probably should unmask the call to loop, if you go that route), or perhaps just put mask_ around the entire loop.

https://mail.haskell.org/pipermail/libraries/2019-December/030134.html

I think just putting mask_ around the entire loop is ok: if something is blocked on the retry or blocked on threadDelay, async exceptions can still happen, and it looks like those opportunities for async exceptions to occur are guaranteed to happen on a regular basis.

Though, to be honest, I think I'd also prefer to modify ignoreExceptions to not eat asynchronous exceptions.

[snoyberg]

No objection to such changes. Interested in sending a PR?

[lpsmith]

I'll try to get a PR sometime this week; though, it occurs to me that if you don't want to put a mask_ around the entire loop, you really need join . mask_ . join . atomically, the method I was thinking of isn't properly tail recursive without more compiler magic than I'm comfortable relying on.