flushTBQueue is broken since 2.5.2.0

[nikita-volkov]

It acts weirdly. Causes random halts and etc.

One issue that is easily reproducible is that the following test fails:

    describe "TBQueue" do
      describe "flushTBQueue" do
        it "Affects the length" do
          queue <- newTBQueueIO 100
          length <- atomically $ do
            writeTBQueue queue 1
            writeTBQueue queue 2
            flushTBQueue queue
            lengthTBQueue queue
          shouldBe length 0

Replacing the implementation with the following suboptimal one can serve as a quick fix for the time being:

flushTBQueue :: TBQueue a -> STM [a]
flushTBQueue queue =
  go []
  where
    go !acc = do
      element <- tryReadTBQueue queue
      case element of
        Just element -> go $ element : acc
        Nothing -> return $ reverse acc

I guess the bug was introduced in #70. So @konsumlamm please take a look.

---

@bgamari @simonmar @hvr

We need stricter quality assurance standards for this package. Bugs in packages as central as this can have very dire impact on the stability of the whole ecosystem and the reputation of the language. There's 13496 indirect dependencies just on Hackage, meaning that virtually any application can get affected by bugs in this package. Another issue is that people don't expect bugs from such central packages, e.g., I've lost quite some time debugging this simply because I could not believe that the bug would come from "stm".

I suggest that rewrites should be done with extreme caution and should be exhaustively covered with tests. As I see #70 came with 0 tests.

[nikita-volkov]

Here's an idea on how the new implementation can be tested. The old implementation can be copied to a test suite which will generate random actions and execute them on both implementations and compare the results. Thus we can ensure that the new implementation behaves the same as the old one.

[mitchellwrosen]

Could the affected versions please be marked as deprecated on Hackage? 🙏

[erikd]

@mitchellwrosen What versions need to be deprecated? Everything >= 2.5.2 && <= 2.5.3 ?

[erikd]

If I get confirmation on the versions to deprecate I can do it as a Hackage Trustee.

[nikita-volkov]

@mitchellwrosen What versions need to be deprecated? Everything >= 2.5.2 && <= 2.5.3 ?

2.5.2.0 and 2.5.2.1

[erikd]

Deprecated done

[andreasabel]

Should this issue be closed now?