Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readLoop hangs if there is an error without a pending reply #12

Open
marcushg opened this issue Mar 14, 2016 · 1 comment
Open

readLoop hangs if there is an error without a pending reply #12

marcushg opened this issue Mar 14, 2016 · 1 comment

Comments

@marcushg
Copy link

Problem
I believe I stumbled onto the same/related problem as #9 (and maybe #6): Whenever my event handling code causes an X11 Error (like mapWindow causing a BadWindow) it seems like no further errors or events are handled by my xhb client (and yes, I run the error and event handler separate from the main "thread" using forkIO).

Hand-waivy explanation
It seems there are two types of requests - those expecting a reply (like listExtensions) and those who don't (like mapWindow). The root cause of the problem seems to lie within the readLoop and their corresponding readLoopError (and readLoopReply) functions. Whenever an error/reply/event is read from the connection it's fed to either readLoopError/readLoopReply/readLoopEvent synchronously. Now, in case of an error readLoopError will try to check if the error is in reply of a previous request (by using readTChan one the pending replies queue, which will block if the queue is empty!). And there lies the problem - when I am sending a mapWindow (i.e. a request without expecting a reply) and cause an error, the readLoopError function will hang indefinitely at readTChan (until you do a request which is expecting a reply, which will fill up the "pending reply" queue) and thus won't return to the eventLoop and in turn block all event/error handling.

Steps to reproduce
It's quite easy to reproduce using xhb's Demo.hs, simply change the handleEvent function to produce an error and you'll only see the first event (likely EnterNotify) and then it'll hang:

-- print events from the queue
handleEvent ::X.Connection -> IO ()
handleEvent c = do
  forkIO $ forever $ do
      e <- X.waitForEvent c
      putStrLn $ showEvent e
      -- deliberately cause an error by mapping an invalid window
      wid <- X.newResource c
      X.mapWindow c wid
      -- from here on the readLoop will "hang"
      -- to unblock it, simply send a dummy request
      -- which expects a reply, like this:
      -- _ <- X.listExtensions c
      return ()

Possible solution
Instead of using readTChan in readLoopError (and maybe readLoopReply) use tryReadTChan in order to not block the readLoop on an empty pending reply queue. In case of tryReadTChan returning Nothing simply push the error to the error queue and return. So far it seems to work with: requests not expecting a reply, requests expecting a reply and also expecting a reply but causing an error.

Not sure of all edge cases though, I've only dug deep enough to fix my own use cases.

@s9gf4ult
Copy link

s9gf4ult commented Jun 5, 2016

I would suggest to use TVar (Map SequenceId RawReceipt) for field conn_reps to prevent any locks. Or even use http://hackage.haskell.org/package/stm-containers-0.2.13/docs/STMContainers-Map.html instead of Data.Map.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants