Proposal: Replace QSem, QSemN, SampleVar in base

[Simon Marlow]

On 12/04/11 14:48, Chris Kuklewicz wrote:
> This is my first proposal to change some things in "base" and I trust
> that my request is not going to be perfect.
>
> The packages that I am proposing to change are all in Control.Concurrent
> and build on MVars: QSem, QSemN, and SampleVar.  I wish to change them
> because this will fix an important bug they have in common.  Who needs
> to approve of the following proposed change?
>
> Additional URLS:
> http://hackage.haskell.org/trac/ghc/ticket/3160
> http://hackage.haskell.org/package/SafeSemaphore
> http://haskell.org/haskellwiki/SafeConcurrent
>
> All three of QSem, QSemN, and SampleVar create an abstraction and
> attempt to maintain an invariant or behavior.  QSem and QSemN present
> the abstraction of a conserved quantity.  SampleVar wishes to maintain a
> single updatable value.
>
> All three have in common that waiting on a semaphore or read the sample
> var will block if there is no quantity or value present.  There can be
> many thread blocked in this way.
>
> All three abstractions fail to maintain their integrity in the case
> where one of the blocked waiters gets interrupted.  This is not a random
> or rare failure, they abstraction always fails after the first waiter
> dies.  Thus the root cause of the problem is that these modules were
> written with the unenforced and undocumented assumption that none of the
> waiters can be interrupted.
>
> The specific failure with QSem and QSemN is that signaling the semaphore
> will pass quantity to the already interrupted waiter.  This quantity
> leak cannot be prevented by the user of module with clever exception
> handling.  This leak will likely cause some or all future attempts to
> wait on the semaphore to block indefinitely.
>
> The specific failure with SampleVar is that writeSampleVar and
> emptySampleVar will trust the no longer correct count of blocked
> readers.  These methods can then block indefinitely.  This is triggered
> by: newEmptySampleVar, readSampleVar, kill the blocked reader,
> writeSampleVar, writeSampleVar.  The writeSampleVar should never more
> than momentarily block, so this indefinite block turns the SampleVar
> into acting like a regular MVar.  Also, isEmptySampleVar will no longer
> ever read full once the SampleVar is broken.
>
> All of these three modules can be replaced with MSem, MSemN, and
> MSampleVar in the SafeSemaphore package (on hackage).  These allow the
> same operations as those in base but without the above flaw.  If a
> waiter dies in the new modules then the waiter will have had no effect
> on the quantity or value.  Slightly surprisingly, these improved modules
> all have simpler implementations.  I have also expanded the APIs to
> expose additional useful operations that can be performed.  Finally, I
> have changed the names in the new modules to be less verbose as the
> community style has evolved to use qualified imports.
>
> There is a vanishingly small change someone depends on the flawed
> behavior of the base modules.  I propose to ignore this possibility and
> replace the old modules with thin layers around SafeSemaphore.

I don't think there's any need to introduce new types.  Just fix the 
exception safety in the existing implementations.  Could you make a patch?

Cheers,
	Simon