ghcjs / jsaddle

JavaScript interface that works with GHCJS or GHC
116 stars 62 forks source link

jsaddle-warp browser DOS on the Haskell program (stuck on sendTextData) #97

Open oliver-batchelor opened 5 years ago

oliver-batchelor commented 5 years ago

I'm writing a program which draws many things on a canvas - and have found that if I draw too many things, too fast, then jsaddle-warp will freeze up. It works fine in ghcjs in-browser. A reduced version is below.

module Main where

import Control.Monad (forM_)
import Data.Monoid ((<>))
import Control.Monad.IO.Class (MonadIO(..))
import Control.Concurrent (forkIO)
import Control.Concurrent.MVar (takeMVar, putMVar, newEmptyMVar)
import Control.Lens ((^.))
import Language.Javascript.JSaddle
       (jsg, jsg3, js, js1, jss, fun, valToNumber, syncPoint,
        nextAnimationFrame, runJSM, askJSM, global)
import Language.Javascript.JSaddle.Warp (run, debug)
import Language.Javascript.JSaddle.Run (enableLogging)

main = run 3708 $ do
    enableLogging True
    doc <- jsg "document"
    doc ^. js "body" ^. jss "innerHTML" ("<h1>Kia ora (Hi)</h1>")

    -- Create a haskell function call back for the onclick event
    doc ^. jss "onmousemove" (fun $ \ _ _ [e] -> do
        x <- e ^. js "clientX" >>= valToNumber
        y <- e ^. js "clientY" >>= valToNumber

        forM_ [1..10000] $ \_ ->
            doc ^. js "body" ^. jss "innerHTML" ("<h1>Kia ora (Hi)" <> show (x, y) <> "</h1>")

        liftIO $ print (x, y)
        return ())
oliver-batchelor commented 5 years ago

It seems to get stuck in this part of the loop in jsaddle.js, where batch[2] is always equal to expectedBatch - 1. When it is responsive it is always batch[2] == expectedBatch.

       if(batch[2] === expectedBatch - 1) {
                batch = (function(){
                       var xhr = new XMLHttpRequest();
                       xhr.open('POST', '/sync/'+syncKey, false);
                       xhr.setRequestHeader("Content-type", "application/json");
                       xhr.send(JSON.stringify({"tag": "BatchResults", "contents": [lastResults[0], lastResults[1]]}));
                       return JSON.parse(xhr.response);})();
              }
oliver-batchelor commented 5 years ago

If I pause the javascript debugger and resume it, it becomes un-stuck for a while.

Looks like the xhr.response after the point it becomes stuck is always the same.

oliver-batchelor commented 5 years ago

I've discovered the Haskell program is stuck on sendTextData, it keeps receiving results (the same results over and over, maybe this is incorrect?) in processResults BatchResults n br -> putMVar recvMVar (n, br) - yet takeResults never sees the result, because the program is stuck in sendBatch (and sendTextData).

It seems like the browser is flooding the Haskell program and it never has a chance to send it's data, and when I stop it with the debugger - it has a chance to catch up.

oliver-batchelor commented 5 years ago

Ok, so I think this is actually a resource starvation problem and not a websockets issue. It does not occur immediately on a big batch of commands - but only with sustained numbers, eventually this fills up some system buffer and Socket.send blocks.

The stuck sendBatch means that the Haskell side does not receive new xhr results. The BatchResults are read by processResult however because sendBatch is blocking, the results are not actually used (takeResult recvMVar nBatch is sitting after sendBatch which is blocking).

For whatever reason there's nothing to check that xhr result batch number is not the same one is last time, so it keeps sending the same xhr and set of results over and over.

A hack seems to be to use forkIO $ sendBatch ..., it seems to work but I'm not sure if it's safe! A better solution would probably be to prevent sending the same results or request twice, either from Javascript side or Haskell side?

hamishmack commented 4 years ago

@saulzar thanks heaps for looking into this. What OS and browser were you testing with? Do you remember if you had this commit.

A better solution would probably be to prevent sending the same results or request twice, either from Javascript side or Haskell side?

We need to make sure we don't discard any at the receiving end, but this does sound like a good plan.

oliver-batchelor commented 4 years ago

@hamishmack - Chrome 72 on Linux Ubuntu 18.04, I believe the version I was testing against had that patch. The modified version I've been using is 0.9.6 (which includes that patch). I will test again and confirm.