Re: mask in waitQSem

2014-11-25 Thread Simon Marlow 

On 25/11/2014 14:03, Yuras Shumovich wrote:

On Tue, 2014-11-25 at 13:53 +, Simon Marlow wrote:

On 24/11/2014 11:12, Yuras Shumovich wrote:

On Mon, 2014-11-24 at 10:09 +, 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.


Well, that's the case for *any* side-effecting operation outside of 
mask: when an exception is raised, you have no idea whether the effect 
took place or not.  That's why we have mask.


It might still be useful outside mask, if the QSem is being used locally 
and it's ok to discard it along with the computation.


Cheers,
Simon



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

Re: mask in waitQSem

2014-11-25 Thread Yuras Shumovich 
On Tue, 2014-11-25 at 13:53 +, Simon Marlow wrote:
> On 24/11/2014 11:12, Yuras Shumovich wrote:
> > On Mon, 2014-11-24 at 10:09 +, 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

Re: mask in waitQSem

2014-11-25 Thread Simon Marlow 

On 24/11/2014 11:12, Yuras Shumovich wrote:

On Mon, 2014-11-24 at 10:09 +, 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.


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

Re: mask in waitQSem

2014-11-24 Thread Yuras Shumovich 
On Mon, 2014-11-24 at 10:09 +, 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.

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

Re: mask in waitQSem

2014-11-24 Thread Simon Marlow 

On 14/11/2014 21:30, Brandon Allbery wrote:

On Fri, Nov 14, 2014 at 4:23 PM, Yuras Shumovich mailto:shumovi...@gmail.com>> wrote:

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 thought QSem was known to be completely unsafe and
http://hackage.haskell.org/package/SafeSemaphore was recommended
instead, with QSem and friends slated for removal?


FUD! :)

I revamped it for 7.6 rather than remove it, it's safe and fast now.

Cheers,
Simon



In any case, there are probably leftover attempts to make it safe, yes.

--
brandon s allbery kf8nh   sine nomine associates
allber...@gmail.com  ballb...@sinenomine.net

unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net


___
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

Re: mask in waitQSem

2014-11-24 Thread Simon Marlow 

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_.


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

Re: mask in waitQSem

2014-11-14 Thread Yuras Shumovich 
On Fri, 2014-11-14 at 16:30 -0500, Brandon Allbery wrote:
> On Fri, Nov 14, 2014 at 4:23 PM, Yuras Shumovich 
> wrote:
> 
> > 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 thought QSem was known to be completely unsafe and
> http://hackage.haskell.org/package/SafeSemaphore was recommended instead,
> with QSem and friends slated for removal?
> 
> In any case, there are probably leftover attempts to make it safe, yes.
> 

Oh, that explains it. Thank you a lot.

Yuras



___
ghc-devs mailing list
ghc-devs@haskell.org
http://www.haskell.org/mailman/listinfo/ghc-devs

Re: mask in waitQSem

2014-11-14 Thread Brandon Allbery 
On Fri, Nov 14, 2014 at 4:23 PM, Yuras Shumovich 
wrote:

> 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 thought QSem was known to be completely unsafe and
http://hackage.haskell.org/package/SafeSemaphore was recommended instead,
with QSem and friends slated for removal?

In any case, there are probably leftover attempts to make it safe, yes.

-- 
brandon s allbery kf8nh   sine nomine associates
allber...@gmail.com  ballb...@sinenomine.net
unix, openafs, kerberos, infrastructure, xmonadhttp://sinenomine.net
___
ghc-devs mailing list
ghc-devs@haskell.org
http://www.haskell.org/mailman/listinfo/ghc-devs