On Tue, 2014-11-25 at 13:53 +0000, Simon Marlow wrote: > On 24/11/2014 11:12, Yuras Shumovich wrote: > > On Mon, 2014-11-24 at 10:09 +0000, Simon Marlow wrote: > >> On 14/11/2014 21:23, Yuras Shumovich wrote: > >>> > >>> I was reviewing some exception handling code in base library, and I > >>> found this line: > >>> https://phabricator.haskell.org/diffusion/GHC/browse/master/libraries/base/Control/Concurrent/QSem.hs;165072b334ebb2ccbef38a963ac4d126f1e08c96$74 > >>> > >>> Here mask is used, but I looks completely useless for me. waitQSem > >>> itself should be called with async exceptions masked, otherwise there is > >>> no way to prevent resource leak. > >>> > >>> Do anyone know why mask is used here? > >> > >> I wrote that code. It looks like mask_ is important, otherwise an async > >> exception might leave the MVar empty, no? We can't assume that waitQSem > >> is always called inside a mask_, so it does its own mask_. > > > > Hmm, yes, you are right. > > > > Note, that in case of failure (and without external `mask`) it is not > > possible to determine, whether a unit of resource was reserved or not. > > It makes the particular instance of `QSem` unusable anymore, and we have > > to create other one. Probably there are situations where it is OK > > though. > > I don't think that's true: if waitQSem fails, then the semaphore has not > been acquired. This guarantee is important for "bracket waitQSem > signalSQem" to work. I just peered at the code again and I believe this > is true - have you spotted a case where it might be false? If so that's > a bug.
The note was about the case when waitQSem is *not* called inside mask (as you said we can't assume that). In that case it can be interrupted by async exception either before entering mask_ or after leaving it. Is it true? That makes waitQSem not very useful outside bracket. When used inside bracket, it seems to be safe. > > Cheers, > Simon > > > > > > Thanks, > > Yuras > > > >> > >> Cheers, > >> Simon > >> > >> > >>> I wonder whether an author of the code tried to do something different, > >>> so there actually can be a bug hidden here. Probably > >>> uninterruptibleMask_ should be used here? (I don't think so though.) > >>> > >>> Thanks, > >>> Yuras > >>> > >>> > >>> _______________________________________________ > >>> ghc-devs mailing list > >>> ghc-devs@haskell.org > >>> http://www.haskell.org/mailman/listinfo/ghc-devs > >>> > > > > _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs