Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixed timeout bug when reading message body #21

Merged
merged 1 commit into from

2 participants

@basvandijk

In Snap.Internal.Types there's this function:

runRequestBody iter = do
    bumpTimeout <- liftM ($ 5) getTimeoutAction
    ...

However instead of bumping (tickle) the timeout with 5 seconds, bumpTimeout will actually incorrectly hard set it to 5 second.

I solved this bug together with @Lemmih.

@basvandijk basvandijk Fixed timeout bug when reading message body
In Snap.Internal.Types there's this function:

runRequestBody iter = do
    bumpTimeout <- liftM ($ 5) getTimeoutAction
    ...

However instead of bumping (tickle) the timeout with 5 seconds,
bumpTimeout will actually incorrectly hard set it to 5 second.

I solved this bug together with David Himmelstrup.
c1568a1
@basvandijk

Here's a way to trigger the bug:

{-# LANGUAGE OverloadedStrings #-}

module Main where

import Control.Monad.IO.Class (liftIO)
import Control.Concurrent (threadDelay)

import Snap

main :: IO ()
main = serveSnaplet defaultConfig appInit

data App = App

appInit :: SnapletInit App App
appInit = makeSnaplet "timeout" "Timeout" Nothing initializer
    where
      initializer :: Initializer App App App
      initializer = do
        addRoutes [("/timeout/", method POST handleTimeout)]
        return App

handleTimeout :: Handler App App ()
handleTimeout = do
  readRequestBody 2000000
  liftIO $ threadDelay (20 * 1000000)

If you POST to /timeout you will get the following error after 5 seconds:

$ time curl -X POST localhost:8000/timeout/ -d somedata
A web handler threw an exception. Details:
thread killed
real    0m5.041s
user    0m0.008s
sys 0m0.004s

Note that commenting the readRequestBody gets rid of the error.

Also I would really like to get a better error message than "thread killed". Something like "timeout exception" would be much better!

@gregorycollins gregorycollins merged commit 35d7a07 into snapframework:master
@gregorycollins gregorycollins referenced this pull request from a commit
@gregorycollins gregorycollins Revert "Merge pull request #21 from basvandijk/fixedTimeoutbug"
This breaks stuff on 0.7 and I'm too lazy to fix it on the stability branch
(likely never to go out?).

This reverts commit 35d7a07, reversing
changes made to d371cc9.
2d2626f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 9, 2012
  1. @basvandijk

    Fixed timeout bug when reading message body

    basvandijk authored
    In Snap.Internal.Types there's this function:
    
    runRequestBody iter = do
        bumpTimeout <- liftM ($ 5) getTimeoutAction
        ...
    
    However instead of bumping (tickle) the timeout with 5 seconds,
    bumpTimeout will actually incorrectly hard set it to 5 second.
    
    I solved this bug together with David Himmelstrup.
This page is out of date. Refresh to see the latest.
Showing with 2 additions and 3 deletions.
  1. +2 −3 src/Snap/Internal/Http/Server/SimpleBackend.hs
View
5 src/Snap/Internal/Http/Server/SimpleBackend.hs
@@ -120,7 +120,7 @@ acceptThread defaultTimeout handler tmgr elog cpu sock exitMVar =
go = runSession defaultTimeout handler tmgr sock
- acceptHandler =
+ acceptHandler =
[ Handler $ \(e :: AsyncException) -> throwIO e
, Handler $ \(e :: SomeException) -> do
elog $ S.concat [ "SimpleBackend.acceptThread: accept threw: "
@@ -160,7 +160,6 @@ runSession defaultTimeout handler tmgr lsock sock addr = do
let sinfo = SessionInfo lhost lport rhost rport $ Listen.isSecure lsock
timeoutHandle <- TM.register (killThread curId) tmgr
- let setTimeout = TM.set timeoutHandle
let tickleTimeout = TM.tickle timeoutHandle
bracket (Listen.createSession lsock 8192 fd
@@ -182,7 +181,7 @@ runSession defaultTimeout handler tmgr lsock sock addr = do
writeEnd
(sendFile lsock (tickleTimeout defaultTimeout)
fd writeEnd)
- setTimeout
+ tickleTimeout
)
Something went wrong with that request. Please try again.