jsre: support for setTimeout, sendAsync

[zsfelfoldi]

using a modified version of https://github.com/robertkrimen/natto/blob/master/natto.go
also removed provider polling from Ethereum_JS
added natto test to jsre_test

[zsfelfoldi]

#772

[fjl]

I don't see a reason why this should be in a separate package. This is purely internal, please move it to package jsre and unexport it.

[zsfelfoldi]

done

[fjl]

I think this doesn't quite do what we want in the console. The console works like this:

for {
    code := readLine()
    result := jsre.Run(code)
    print(result)
}

With the 'natto' approach so far, the second step would simply hang if the JS code installed any timers.
In the console, we need an approach where code can be injected into the event loop and also need to add a facility to abort the event loop at any time. This can be done by structuring the event loop as follows:

type evalReq struct {
    code string
    result *otto.Value
    err  error
    done chan struct{}
}

loop:
for {
    select {
    case timer := <-timerReady:
        // execute callback, remove/reschedule the timer
    case evalReq := <-eval:
        // run the code, send the result back
        evalReq.result, evalReq.err = vm.Run(evalReq.code)
        close(evalReq.done)
    case <-stop:
        break loop
    }
}

Note that this loop never stops unless explicitly asked. The stop clause would need to be extended
so it can optionally wait until there are no more timers (basically the natto behaviour you have now). We need this when executing code from a file (from the geth js command).

[fjl]

I think the proper fix for ethereum.js would be to ensure that it removes all of its timers and intervals when all filters (and whisper watches, etc.) are deregistered. This would provide a way for the application to exit cleanly when running code from a file. The change should be made in the ethereum.js repository (it's not a bad thing to have).

[fjl]

When writing "please remove this comment", I meant "please remove the package comment".

[fjl]

This is still missing the 'extended' stop logic for geth js.

Here is an example you can try both in the console and when executing from a file:

function callback(n) {
    console.log("callback", n)
    if (n > 0) {
        setTimeout(callback, 1, n - 1);
    }
}

setTimeout(callback, 1, 10);

When you run this in geth console, it should be possible to exit the console immediately by typing exit<Return> or ^D. The console should not wait for callbacks.

When running with geth js test.js (assuming you put the above code into test.js), the program should exit after 10 seconds.

[fjl]

My suggestion for the API is adding a parameter to JSRE.Stop:

func (*JSRE) Stop(waitForCallbacks bool)

[zsfelfoldi]

Fixed the above issues, thanks for the review.

[fjl]

Please submit an issue to web3.js. It should be fixed to remove timers when there is nothing to poll.

[fjl]

Please change loadScript to use the vm directly instead. This is fine because it can only be called from JS code, which runs inside the event loop. There should be a comment in loadScript that explains why using the vm directly (without evalQueue) is ok.

[fjl]

Note that having the loop means that self.vm cannot be used from another goroutine. All calls that use the VM need to go through the evalQueue instead. You can address this by changing evalReq so it carries a func() instead.

[fjl]

A few overall comments:

Documentation: Please document Stop and its new parameter using godoc comments.
It would also not hurt to document (above New and maybe on JSRE) that the runtime must
be stopped after use and cannot be used after stopping.

godoc comments use the following form (name of function as first word, full sentence, no /* */):

// Method does something and is very awesome.
func (MyCoolType) Method() foo {
    ...
}

Formatting: Please remove blank lines at the beginning of functions and the weird blank line between imports. The imports should have two blank-separated sections, one for stdlib imports and one for the rest.

Commit message: It is hard for people to see what a commit with message "1" is about. Please squash your jsre changes into a single commit that says something about the content, such as "jsre: add support for setTimeout and setInterval". The web3.js change should be a separate commit.

Thank you.

[obscuren]

Accidental label removal

[zsfelfoldi]

#814 is also needed for the js filters.

[fjl]

What is the status of this PR? Are you still planning to address the issues above?

[zsfelfoldi]

Fixed the above issues today.

[obscuren]

Exported functions like Exec and Set/Get should not expose the bypassing functionality of the eval loop. There need to be two function:

• exec(file string, useEQ bool) error
• Exec(file string) error

Exec Will always use the eval loop where as exec it's optional (and required)

[obscuren]

Exported functions need to be documented

[fjl]

I believe that the parameter was added so Go functions called from JS can bypass the event loop since
they already run from inside the loop. Sending to the eval channel would hang in those functions.

As an idea for solving this, maybe Bind should be modified to pass the *otto.VM as the first parameter
to Go functions. The VM could then be used directly inside of the callback.

re := jsre.New(...)
re.Bind("foo", func (vm *otto.Otto, call otto.FunctionCall) otto.Value {
    ...  
})

In fact, this could clean up the API a bit because there is currently no difference
between Bind and Set. The case where this might not be so easy to implement is when Go
binding functions are passed deep inside a data structure (e.g. in map[string]interface{}).

[fjl]

Looking further, otto.FunctionCall already contains the VM as a public field.
I would like to request that the WithoutEQ versions are removed and binding functions use
the VM from the call struct instead.

[fjl]

This is looking much better now. Thank you. There are minor details that could be cleaned up
but we can do that later.