jessedoyle / duktape.cr

Evaluate JavaScript from Crystal!
MIT License
137 stars 17 forks source link

re-using sandbox/context #53

Closed sam0x17 closed 5 years ago

sam0x17 commented 5 years ago

Is there a way I can call multiple eval!'s without losing the previous state? This would help me save significantly on runtime. e.g. I want to do one eval! that loads a js library, and then periodically re-use that sandbox, doing a new eval! call that uses the library, without having to re-load everything every time.

sam0x17 commented 5 years ago

To clarify, right now when I try calling a method that should have been loaded by the previous eval!, it says the method is undefined, but if I concatenate everything into one eval! call it works.

jessedoyle commented 5 years ago

Hi @sam0x17, thanks for using duktape.cr!

Duktape does store state on its stack between subsequent evaluation calls. Are you sure that the initial assignment is succeeding?

For example, consider the following code:

require "duktape"

sbx = Duktape::Sandbox.new
sbx.eval! "var state = { test: true };"
sbx.eval! "print('State from previous eval: ' + state.test);"

This code will output the following:

State from previous eval: true

To help debug, it's possible to dump the Duktape stack at any time. For example:

sbx = Duktape::Sandbox.new
sbx.dump!
sbx.eval!("var state = { test: function() { return 'test'; } };")
sbx.dump!
sbx.eval!("print('State from previous eval: ' + state.test());")
sbx.dump!
sbx.eval!("1 + 1")
sbx.dump!

will output:

STACK: ctx: top=0, stack=[]
STACK: ctx: top=1, stack=[undefined]
State from previous eval: test
STACK: ctx: top=2, stack=[undefined,undefined]
STACK: ctx: top=3, stack=[undefined,undefined,2]

where the undefined stack values are returned from variable assignment and the print function call.

Would you be willing to share a bit more context around your particular issue? It would be ideal if we could reproduce it.

Thanks!

sam0x17 commented 5 years ago

The problem I'm having is globals don't seem to persist between calls to eval. Here is my code:

require "duktape/runtime"
require "baked_file_system"

class Uglify
  extend BakedFileSystem
  bake_folder "../uglify"
end

UGLIFY_ITEMS = [:minify, :utils, :ast, :parse, :transform, :scope, :output, :compress, :propmangle]
UGLIFY_JS = UGLIFY_ITEMS.map { |item| Uglify.get("#{item}.js").gets_to_end }.join("\n")

module JsMinifier
  @@ctx = Duktape::Runtime.new { |ctx| ctx.eval!(UGLIFY_JS) }

  def self.minify(source : String)
    source = source.gsub("\"", "\\\"")
    source = source.gsub("\n", "\\n")
    @@ctx.eval("var res = minify(\"#{source}\");\n")
    res = @@ctx.call("res")
  end
end

JsMinifier.minify("test();")

This results in:

Unhandled exception: ReferenceError: identifier 'minify' undefined (Duktape::ReferenceError)
  from lib/duktape/src/duktape/api/error_handling.cr:47:13 in 'raise_error'
  from lib/duktape/src/duktape/api/eval.cr:122:7 in 'eval_string!'
  from lib/duktape/src/duktape/api/eval.cr:32:7 in 'eval!'
  from lib/duktape/src/duktape/runtime.cr:143:7 in 'eval'
  from src/js-minifier.cr:10:5 in 'minify'
  from src/js-minifier.cr:15:1 in '__crystal_main'
  from /usr/local/Cellar/crystal/0.31.0/src/crystal/main.cr:97:5 in 'main_user_code'
  from /usr/local/Cellar/crystal/0.31.0/src/crystal/main.cr:86:7 in 'main'
  from /usr/local/Cellar/crystal/0.31.0/src/crystal/main.cr:106:3 in 'main'

However if I do everything in one eval call (instead of the later call), it works fine, and minify is not an undefined reference.

sam0x17 commented 5 years ago

note: repo for this code is here: https://github.com/sam0x17/js-minifier

jessedoyle commented 5 years ago

@sam0x17 - Thanks for the example. I'll dig in over the next few days and hopefully we can figure out the issue!

sam0x17 commented 5 years ago

A more minimal example:

    ctx = Duktape::Sandbox.new
    ctx.eval!(UGLIFY_JS + "\nprint(minify);\n")
    ctx.eval!("print(minify);\n")

Results in a reference error for minify on the second eval, however the first print works. If I do a dummy function like function minify() { return true; } instead, it works and there is no reference error. So for whatever reason, minify references disappear once you exit the eval in which minify is defined.

I also tried doing things like var minify = minfiy; and seeing if that tricked it into persisting, but that did not help -- same behavior.

sam0x17 commented 5 years ago

Also, the minify function is defined here: https://github.com/sam0x17/js-minifier/blob/master/uglify/minify.js#L53

jessedoyle commented 5 years ago

Okay, so I've been able to start looking into the problem. I can replicate the issue by using your repository - I'm still not 100% sure what the root cause is.

We do have a spec case that loads UglifyJS and runs a sample string through it (see link). If you check the spec out, you'll see it's working as expected - and it uses 2 separate eval calls.

I think the issue may be related to how js-minifier is loading the JS code by reading files individually and concatenating the contents together (but I could be very wrong).

I'll keep looking into it - just wanted to point you to the spec case above as an example!

jessedoyle commented 5 years ago

Minimal reproduction:

sbx = Duktape::Sandbox.new
sbx.eval! <<-JS
  "use strict";
  function test(arg) { return arg; }
JS
sbx.eval!("test(1);") # ReferenceError: identifier 'test' undefined

It appears that the issue is that the sources that your library pull in all begin with the use strict directive.

I'm not sure if this behaviour is intentional or not, but the use strict directives are preventing the evaluation calls from sharing state.

jessedoyle commented 5 years ago

FYI: removing the use strict directive on the all the individual JS sources in js-minifier resolves the issue for me.

I'm not sure if you've found a bug in Duktape's strict mode implementation. It's very possible this is intentional behaviour, but I can't find any reference to it in this document.

It may be worthwhile opening an issue in https://github.com/svaarala/duktape to determine if this is intentional behaviour.

jessedoyle commented 5 years ago

I've created an upstream issue here.

For now, we'll keep this issue open as we track the progress.

sam0x17 commented 5 years ago

ahhh I slightly suspected that might be it -- thanks!! for my purposes, I can just remove all but one of the use stricts

jessedoyle commented 5 years ago

@sam0x17 - See the comment here: https://github.com/svaarala/duktape/issues/2186#issuecomment-536345674

This behaviour is intentional when using strict mode in the engine (and in fact node behaves similarly).

As we have bindings to almost all Duktape API calls, you may be able to use a combo of compile_string! and call to force the interpreter to operate in a non-strict context. I wouldn't recommend that as it's likely not worth the extra complexity.

I would suggest simply removing the use strict directives where possible, or running the JS code through a packager (babel, etc.) before running it via Duktape.