informatikr / hedis

A Redis client library for Haskell.
http://hackage.haskell.org/package/hedis
BSD 3-Clause "New" or "Revised" License
329 stars 127 forks source link

thread blocked indefinitely in an MVar operation #17

Closed twittner closed 8 years ago

twittner commented 10 years ago

Please consider the following test case:

{-# LANGUAGE OverloadedStrings #-}

module Main where

import Control.Monad
import Database.Redis

main :: IO ()
main = do
    r <- connect defaultConnectInfo { connectPort = PortNumber 9042 }
    void $ runRedis r $
        replicateM 2000 $ do
            x <- expire "hello" 10
            return x

Execution yields (potentially after multiple runs):

$ ghc --make -O2 -threaded -Wall -fforce-recomp testcase
[1 of 1] Compiling Main             ( testcase.hs, testcase.o )
Linking testcase ...
$ ./testcase 
testcase: ConnectionLost
testcase: thread blocked indefinitely in an MVar operation

The ConnectionLost exception was provoked by the fact that there isn't actually a Redis server listening on port 9042 but a different service (Cassandra). The test case is setup this way to highlight two aspects:

  1. All parsing failures are interpreted as connection loss.
  2. If the evaluation thread gets to evaluate errConnClosed it is terminated immediately, i.e. there is no longer a consumer of the bounded channel connThunks.

The second point in turn leads to the BlockedIndefinitelyOnMVar exception if the bounded channel becomes full. This can happen if the results of recv calls are not evaluated fast enough within runRedis. If we change the test case to force evaluation to HNF we get instead:

{-# LANGUAGE OverloadedStrings #-}

module Main where

import Control.Monad
import Database.Redis

main :: IO ()
main = do
    r <- connect defaultConnectInfo { connectPort = PortNumber 9042 }
    void $ runRedis r $
        replicateM 2000 $ do
            x <- expire "hello" 10
            x `seq` return x
$ ghc --make -O2 -threaded -Wall -fforce-recomp testcase
[1 of 1] Compiling Main             ( testcase.hs, testcase.o )
Linking testcase ...
$ ./testcase 
testcase: ConnectionLost
testcase: ConnectionLost

Forcing the evaluation evaluates errConnClosed (= throwIO ConnectionLost) within runRedis immediately which then guarantees that the connection is closed and the evaluation thread killed.

That being said it seems that using seq would destroy the pipelining property so maybe instead the evaluation thread should catch exceptions to guarantee there is always a consumer of connThunks as long as the connection is in use?

qrilka commented 8 years ago

The problem BlockedIndefinitelyOnMVar should be resolved with #45 I'd propose to close this one and add another ticket about more consistent and better structured errors in hedis

twittner commented 8 years ago

Unfortunately the testcase from above gives me the same result with hedis 0.7.1.

qrilka commented 8 years ago

Yep, it looks like a different problem - we've got commands waiting for the BoundedChan but the closed connection abrupts runRedis execution without draining that channel. Thanks for quick answer @twittner