Governed Runtime #7

jeremyjh · 2018-07-03T02:00:25Z

resolves #6

As described in #6, this provides the a basic mechanism to implement eval timeouts using Duktape's DUK_USE_EXEC_TIMEOUT_CHECK feature. That feature is very basic, we pass a macro with the name of an externed function that the Duktape runtime will call periodically.

This is a static binding that must be in the duk_config.h file at the time duktape.c is built and this imposes an unfortunate (distribution) build requirement, which is to invoke Duktape's configure script (requires Python and PyYAML). Invoking dukconfig/build_dist.sh will take it from there. The configuration script tells Duktape to invoke a function we extern.

That function always gets called but just returns False (don't timeout) unless the heap was created with a udata parameter. If there is a udata, it expects that udata to contain a function pointer it can unwrap and evaluate and then respond accordingly.

createGovernedHeap takes a an IO Bool and properly wraps it (and sets up finalizers) so it can be conveyed as udata. There is a test that should make basic usage clear.

valpackett · 2018-07-03T11:43:02Z

dukconfig/build_dist.sh

@@ -0,0 +1,9 @@
+#!/bin/bash


Please use #!/bin/sh instead of GNU/Linux-specific paths like /bin/bash. You're not even using any bash-isms in this tiny script :)

(but anyway, I suggested not using a shell script below, just running the commands from Setup.hs)

valpackett · 2018-07-03T11:43:24Z

dukconfig/build_dist.sh

+       --output-directory dukconfig/dist \
+       --source-directory duktape/src-input \
+       --config-metadata duktape/config \
+       --option-file dukconfig/config.yaml \


Trailing backslash

valpackett · 2018-07-03T11:52:22Z

README.md

@@ -64,7 +64,15 @@ The functions must be of type `IO ()`, `IO Value`, `Value -> IO Value`, `Value -

 ## Development

-Use [stack] to build.  
+Building from the repository requires initialization with the Duktape configuration script the first time (these files are included in the cabal distribution on Hackage). 


Looks like it might be possible to use Setup.hs with hooks to run the configure script?

I thought about that. It takes a non-trivial time to run the script so we don't want to do it as part of every build, and it isn't needed for installation from the sdist package. Maybe a preConf hook that checks to see if the files are already there, and runs the script if not, and a postClean that removes them if it can find configure.py. You'd have to remember to clean after updating the Duktape version though.

Alternatively configure.py also produces a json metadata file; I could look for that file and compare its version to what I find in the duktape repo, and always rebuild if they differ.

I think I'd check if the modification time of the duktape source files is after the modification time of the files produced by configure.

jeremyjh · 2018-07-03T13:04:58Z

I just had another thought. It seems unlikely that someone is already using void* udata for something else, but they could be and this change would break that in segfaulty ways. Probably rather than let a raw (Ptr ()) be passed to createHeap for udata there should be a newtype wrapper.

jeremyjh · 2018-07-05T00:12:55Z

@myfreeweb I've made the described changes. A few things to note:

stack doesn't invoke Cabal's clean hooks, so I've left that out. I don't think its really an issue.

When building from source the first time there will always be cabal warnings that it cannot find required extra files.

I had to include a dummy duktape.h or stack will not even build Setup.hs. Since it is a tracked file git does not ignore it. You can force it to do so (locally) with git update-index --skip-worktree dukconfig/dist/duktape.h

stack sdist complains about custom-setup deps. This is a bug in stack. Cabal complains if you don't include the stanza.

valpackett · 2018-07-05T09:17:20Z

Looks good, thanks!

chrisdone · 2020-03-04T10:56:37Z

library/Scripting/Duktape/Raw.hs

+  -- TimeoutCheck is wrapped to pass as void* udata in `createHeap` and will be provided (by duktape)
+  -- back to `execTimeoutCheck` when the interpreter invokes that callback.
+  wrapTimeoutCheckUData ∷ TimeoutCheck → IO (InternalUData, IO ())
+  wrapTimeoutCheckUData check = do


This code is not safe from async exceptions.

I'm not trying to dismiss this as I'm sure masking is a good idea in general, but for motivation can you give an example of an async exception we could expect and recover from? Or would this be the user of the library throwing one at us while they are building a heap for some reason?

If the user of the lib uses System.Timeout.timeout or Control.Concurrent.Async.race, for example, the gap between any of your IO actions are potential exceptions.

A system under heavy load will timeout more easily. I've seen this with e.g. sandboxing tryhaskell.org.

In the race case, if you do e.g. race evalThis readFromCache - i.e. return whichever completes first, that might throw in your initialisation process.

On a long running service this will at best slowly accumulate allocated bytes. At worst you're looking at increased fragmentation because these are pinned mallocs that can't be moved around by the GC.

I'm doing research for running PureScript's JS output for server-side validation/server-side rendering so I'm evaluating whether to use hs-duktape, write my own binding that will address all my concerns from the ground up, or call out to Nodejs in a pipe and avoid the concerns altogether (and the extra work). 🤔

My two-cents new projects should consider using Fabrice Bellard's quickjs instead though I'm not aware of an existing Haskell binding. I doubt duk will ever have es-6 compatibility. I ended up using quickjs via a Rust binding.

I looked at quickjs, but I don't think it has (1) the maturity required, or (2) long-term viability until someone takes over maintenance. Bellard is great and all but he tends to make things and move onto something else, leaving someone else to maintain. Whereas duktape has a proper community, documentation, a list of projects using it, an official GitHub project with an issue tracker where I can report bugs, open PRs, etc. and the expectation that it'll be maintained in five years. Small JS implementations are already niche.

YMMV.

chrisdone · 2020-03-04T10:58:22Z

test-suite/Scripting/DuktapeSpec.hs

+      rE `shouldBe` Left "RangeError: execution timeout"
+
+
+allowQuarterSecond ∷ IO (IO Bool)


This will add a pretty hard performance penalty over having a separate thread just write a bool and this callback just reading an IORef atomically.

That would be a better example to provide to people. Duktape doesn't invoke this callback very frequently so actual cost is maybe not as bad as you're thinking; I did some benchmarking but don't have the data handy now unfortunately.

chrisdone · 2020-03-04T10:59:28Z

library/Scripting/Duktape/Raw.hs

+createGovernedHeap ∷ FunPtr DukAllocFunction → FunPtr DukReallocFunction → FunPtr DukFreeFunction → TimeoutCheck → FunPtr DukFatalFunction → IO (Maybe DuktapeCtx)
+createGovernedHeap allocf reallocf freef timeoutCheck fatalf = do
+  (udata, release) ← wrapTimeoutCheckUData timeoutCheck
+  mctx ← createHeap allocf reallocf freef udata fatalf


If the thread is killed after line 193, then what you just allocated won't ever be released.

I think this is an issue in createHeap as well for the actual Duktape heap.

jeremyjh added 6 commits July 1, 2018 19:50

Basic build stuff is working.

3e87b0f

Pointer wrappers and createGoverenedHeap

65f3077

Passing tests.

f7efd60

Reformat symbols.

dcefb9e

Update README.md

55d873d

CI needs to know about build_dist

3c50fae

valpackett suggested changes Jul 3, 2018

View reviewed changes

jeremyjh added 5 commits July 4, 2018 19:25

Use cabal custom build to build duktape sources.

da595e8

unicode syntax

db2a33a

add placeholder file

a1e6b29

Newtype wrapper on udata.

5ec0aeb

Update README

b65d33d

valpackett approved these changes Jul 5, 2018

View reviewed changes

valpackett merged commit fa72222 into valpackett:master Jul 5, 2018

jeremyjh deleted the governed-runtime branch July 5, 2018 11:32

chrisdone reviewed Mar 4, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Governed Runtime #7

Governed Runtime #7

jeremyjh commented Jul 3, 2018

valpackett Jul 3, 2018 •

edited

Loading

valpackett Jul 3, 2018

valpackett Jul 3, 2018

jeremyjh Jul 3, 2018

valpackett Jul 3, 2018

jeremyjh commented Jul 3, 2018

jeremyjh commented Jul 5, 2018 •

edited

Loading

valpackett commented Jul 5, 2018

chrisdone Mar 4, 2020

jeremyjh Mar 4, 2020

chrisdone Mar 4, 2020

jeremyjh Mar 4, 2020

chrisdone Mar 4, 2020

chrisdone Mar 4, 2020

jeremyjh Mar 4, 2020

chrisdone Mar 4, 2020

jeremyjh Mar 4, 2020

		rE `shouldBe` Left "RangeError: execution timeout"


		allowQuarterSecond ∷ IO (IO Bool)

Governed Runtime #7

Governed Runtime #7

Conversation

jeremyjh commented Jul 3, 2018

valpackett Jul 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremyjh commented Jul 3, 2018

jeremyjh commented Jul 5, 2018 • edited Loading

valpackett commented Jul 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valpackett Jul 3, 2018 •

edited

Loading

jeremyjh commented Jul 5, 2018 •

edited

Loading