Fix "Slow CreateAccount" #251 (#283)

geth/common/types.go

@@ -179,7 +179,6 @@ type JailCell interface {
 	Set(string, interface{}) error
 	Get(string) (otto.Value, error)
 	Run(string) (otto.Value, error)
-	RunOnLoop(string) (otto.Value, error)
 }
 
 // JailManager defines methods for managing jailed environments

geth/jail/jail.go

@@ -14,8 +14,6 @@ import (
 	"github.com/status-im/status-go/geth/log"
 	"github.com/status-im/status-go/geth/params"
 	"github.com/status-im/status-go/static"
-
-	"fknsrs.biz/p/ottoext/loop"
 )
 
 // FIXME(tiabc): Get rid of this global variable. Move it to a constructor or initialization.
@@ -62,7 +60,7 @@ func (jail *Jail) NewJailCell(id string) (common.JailCell, error) {
 
 	vm := otto.New()
 
-	newJail, err := newJailCell(id, vm, loop.New(vm))
+	newJail, err := newJailCell(id, vm)
 	if err != nil {
 		return nil, err
 	}
@@ -160,15 +158,6 @@ func (jail *Jail) Call(chatID string, path string, args string) string {
 
 	res, err := jcell.Call("call", nil, path, args)
 
-	// WARNING(influx6): We can have go-routine leakage due to continous call to this method
-	// and the call to cell.CellLoop().Run() due to improper usage, let's keep this
-	// in sight if things ever go wrong here.
-	// Due to the new event loop provided by ottoext.
-	// We need to ensure that all possible calls to internal setIntervals/SetTimeouts/SetImmediate
-	// work by lunching the loop.Run() method.
-	// Needs to be done in a go-routine.
-	go jcell.lo.Run()
-
 	return makeResult(res.String(), err)
 }
 

geth/jail/jail_cell.go

@@ -3,10 +3,9 @@ package jail
 import (
 	"sync"
 
-	"fknsrs.biz/p/ottoext/fetch"
-	"fknsrs.biz/p/ottoext/loop"
-	"fknsrs.biz/p/ottoext/timers"
 	"github.com/robertkrimen/otto"
+	"github.com/status-im/ottoext/loop"
+	"github.com/status-im/ottoext/timers"
 )
 
 const (
@@ -21,62 +20,32 @@ type JailCell struct {
 
 	id string
 	vm *otto.Otto
-	lo *loop.Loop
 }
 
 // newJailCell encapsulates what we need to create a new jailCell from the
 // provided vm and eventloop instance.
-func newJailCell(id string, vm *otto.Otto, lo *loop.Loop) (*JailCell, error) {
-	// Register fetch provider from ottoext.
-	if err := fetch.Define(vm, lo); err != nil {
-		return nil, err
-	}
+func newJailCell(id string, vm *otto.Otto) (*JailCell, error) {
+	// create new event loop for the new cell.
+	// this loop is handling 'setTimeout/setInterval'
+	// calls and is running endlessly in a separate goroutine
+	lo := loop.New(vm)
 
-	// Register event loop for timers.
+	// register handlers for setTimeout/setInterval
+	// functions
 	if err := timers.Define(vm, lo); err != nil {
 		return nil, err
 	}
 
+	// finally, start loop in a goroutine
+	// JailCell is currently immortal, so the loop
+	go lo.Run()
+
 	return &JailCell{
 		id: id,
 		vm: vm,
-		lo: lo,
 	}, nil
 }
 
-// Fetch attempts to call the underline Fetch API added through the
-// ottoext package.
-func (cell *JailCell) Fetch(url string, callback func(otto.Value)) (otto.Value, error) {
-	val, err := cell.prepareFetchCall(url, callback)
-	if err != nil {
-		return val, err
-	}
-
-	return val, cell.lo.Run()
-}
-
-// prepareFetchCall prepares the needed calls to hook into the vm to receive the expected response
-// for a call to the FetchAPI. We need this to ensure confidence in mutex locking and unlocking.
-func (cell *JailCell) prepareFetchCall(url string, callback func(otto.Value)) (otto.Value, error) {
-	cell.Lock()
-	defer cell.Unlock()
-
-	if err := cell.vm.Set("__captureFetch", callback); err != nil {
-		return otto.UndefinedValue(), err
-	}
-
-	return cell.vm.Run(`fetch("` + url + `").then(function(response){
-			__captureFetch({
-				"url": response.url,
-				"type": response.type,
-				"body": response.text(),
-				"status": response.status,
-				"headers": response.headers,
-			});
-		});
-	`)
-}
-
 // Set sets the value to be keyed by the provided keyname.
 func (cell *JailCell) Set(key string, val interface{}) error {
 	cell.Lock()
@@ -93,34 +62,6 @@ func (cell *JailCell) Get(key string) (otto.Value, error) {
 	return cell.vm.Get(key)
 }
 
-// RunOnLoop evaluates the giving js string on the associated vm loop returning
-// an error.
-func (cell *JailCell) RunOnLoop(val string) (otto.Value, error) {
-	cell.Lock()
-	defer cell.Unlock()
-
-	res, err := cell.vm.Run(val)
-	if err != nil {
-		return res, err
-	}
-
-	return res, cell.lo.Run()
-}
-
-// CallOnLoop attempts to call the internal call function for the giving response associated with the
-// proper values.
-func (cell *JailCell) CallOnLoop(item string, this interface{}, args ...interface{}) (otto.Value, error) {
-	cell.Lock()
-	defer cell.Unlock()
-
-	res, err := cell.vm.Call(item, this, args...)
-	if err != nil {
-		return res, err
-	}
-
-	return res, cell.lo.Run()
-}
-
 // Call attempts to call the internal call function for the giving response associated with the
 // proper values.
 func (cell *JailCell) Call(item string, this interface{}, args ...interface{}) (otto.Value, error) {

geth/jail/jail_cell_test.go

@@ -1,12 +1,9 @@
 package jail_test
 
 import (
-	"net/http"
-	"net/http/httptest"
 	"time"
 
 	"github.com/robertkrimen/otto"
-	"github.com/status-im/status-go/geth/jail"
 	"github.com/status-im/status-go/geth/params"
 )
 
@@ -14,78 +11,62 @@ func (s *JailTestSuite) TestJailTimeoutFailure() {
 	require := s.Require()
 	require.NotNil(s.jail)
 
-	newCell, err := s.jail.NewJailCell(testChatID)
+	cell, err := s.jail.NewJailCell(testChatID)
 	require.NoError(err)
-	require.NotNil(newCell)
+	require.NotNil(cell)
 
 	// Attempt to run a timeout string against a JailCell.
-	_, err = newCell.RunOnLoop(`
-		setTimeout(function(n){
-			if(Date.now() - n < 50){
-				throw new Error("Timedout early");
-			}
+	_, err = cell.Run(`
+		var timerCounts = 0;
+ 		setTimeout(function(n){		
+ 			if (Date.now() - n < 50) {
+ 				throw new Error("Timed out");
+ 			}
 
-			return n;
-		}, 30, Date.now());
-	`)
+			timerCounts++;
+ 		}, 30, Date.now());
+ 	`)
+	require.NoError(err)
 
-	require.NotNil(err)
+	// wait at least 10x longer to decrease probability
+	// of false negatives as we using real clock here
+	time.Sleep(300 * time.Millisecond)
+
+	value, err := cell.Get("timerCounts")
+	require.NoError(err)
+	require.True(value.IsNumber())
+	require.Equal("0", value.String())
 }
 
 func (s *JailTestSuite) TestJailTimeout() {
 	require := s.Require()
 	require.NotNil(s.jail)
 
-	newCell, err := s.jail.NewJailCell(testChatID)
+	cell, err := s.jail.NewJailCell(testChatID)
 	require.NoError(err)
-	require.NotNil(newCell)
+	require.NotNil(cell)
 
 	// Attempt to run a timeout string against a JailCell.
-	res, err := newCell.RunOnLoop(`
-		setTimeout(function(n){
-			if(Date.now() - n < 50){
-				throw new Error("Timedout early");
-			}
-
-			return n;
-		}, 50, Date.now());
-	`)
-
-	require.NoError(err)
-	require.NotNil(res)
-}
-
-func (s *JailTestSuite) TestJailFetch() {
-	mux := http.NewServeMux()
-	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
-		w.WriteHeader(http.StatusOK)
-		w.Write([]byte("Hello World"))
-	})
-
-	server := httptest.NewServer(mux)
-	defer server.Close()
-
-	require := s.Require()
-	require.NotNil(s.jail)
-
-	newCell, err := s.jail.NewJailCell(testChatID)
-	require.NoError(err)
-	require.NotNil(newCell)
-
-	jcell, ok := newCell.(*jail.JailCell)
-	require.Equal(ok, true)
-	require.NotNil(jcell)
-
-	wait := make(chan struct{})
-
-	// Attempt to run a fetch resource.
-	_, err = jcell.Fetch(server.URL, func(res otto.Value) {
-		go func() { wait <- struct{}{} }()
-	})
+	_, err = cell.Run(`
+		var timerCounts = 0;
+ 		setTimeout(function(n){		
+ 			if (Date.now() - n < 50) {
+ 				throw new Error("Timed out");
+ 			}
 
+			timerCounts++;
+ 		}, 50, Date.now());
+ 	`)
 	require.NoError(err)
 
-	<-wait
+	// wait at least 10x longer to decrease probability
+	// of false negatives as we using real clock here
+	time.Sleep(300 * time.Millisecond)
+
+	value, err := cell.Get("timerCounts")
+	require.NoError(err)
+	require.True(value.IsNumber())
+	require.Equal("1", value.String())
 }
 
 func (s *JailTestSuite) TestJailLoopInCall() {
@@ -121,7 +102,7 @@ func (s *JailTestSuite) TestJailLoopInCall() {
 	`)
 	require.NoError(err)
 
-	_, err = cell.CallOnLoop("callRunner", nil, "softball")
+	_, err = cell.Call("callRunner", nil, "softball")
 	require.NoError(err)
 
 	select {

vendor/github.com/status-im/ottoext/README.md

@@ -1,7 +1,7 @@
 ottoext
 =======
 
-[![GoDoc](https://godoc.org/fknsrs.biz/p/ottoext?status.svg)](https://godoc.org/fknsrs.biz/p/ottoext)
+[![GoDoc](https://godoc.org/github.com/status-im/ottoext?status.svg)](https://godoc.org/github.com/status-im/ottoext)
 
 Overview
 --------

vendor/github.com/status-im/ottoext/cmd/ottoext/main.go

@@ -6,16 +6,16 @@ import (
 	"io"
 	"io/ioutil"
 
-	"fknsrs.biz/p/ottoext/loop"
-	"fknsrs.biz/p/ottoext/loop/looptask"
-	erepl "fknsrs.biz/p/ottoext/repl"
+	"github.com/status-im/ottoext/loop"
+	"github.com/status-im/ottoext/loop/looptask"
+	erepl "github.com/status-im/ottoext/repl"
 	"github.com/robertkrimen/otto"
 	"github.com/robertkrimen/otto/repl"
 
-	"fknsrs.biz/p/ottoext/fetch"
-	"fknsrs.biz/p/ottoext/process"
-	"fknsrs.biz/p/ottoext/promise"
-	"fknsrs.biz/p/ottoext/timers"
+	"github.com/status-im/ottoext/fetch"
+	"github.com/status-im/ottoext/process"
+	"github.com/status-im/ottoext/promise"
+	"github.com/status-im/ottoext/timers"
 )
 
 var (

vendor/github.com/status-im/ottoext/fetch/fetch.go

@@ -1,4 +1,4 @@
-package fetch // import "fknsrs.biz/p/ottoext/fetch"
+package fetch
 
 import (
 	"io"
@@ -10,8 +10,8 @@ import (
 	"github.com/GeertJohan/go.rice"
 	"github.com/robertkrimen/otto"
 
-	"fknsrs.biz/p/ottoext/loop"
-	"fknsrs.biz/p/ottoext/promise"
+	"github.com/status-im/ottoext/loop"
+	"github.com/status-im/ottoext/promise"
 )
 
 func mustValue(v otto.Value, err error) otto.Value {

vendor/github.com/status-im/ottoext/loop/loop.go

@@ -1,4 +1,4 @@
-package loop // import "fknsrs.biz/p/ottoext/loop"
+package loop
 
 import (
 	"fmt"
@@ -144,46 +144,22 @@ func (l *Loop) processTask(t Task) error {
 	return nil
 }
 
-// Run handles the task scheduling and finalisation. It will block until
-// there's no work left to do, or an error occurs.
+// Run handles the task scheduling and finalisation.
+// It runs infinitely waiting for new tasks.
 func (l *Loop) Run() error {
-	for {
-		l.lock.Lock()
-		if len(l.tasks) == 0 {
-			// prevent any more tasks entering the ready channel
-			l.closed = true
-
-			l.lock.Unlock()
-
-			break
+	for t := range l.ready {
+		if t == nil {
+			continue
 		}
-		l.lock.Unlock()
 
-		t := <-l.ready
-
-		if t != nil {
-			if err := l.processTask(t); err != nil {
-				return err
-			}
+		err := l.processTask(t)
+		if err != nil {
+			// TODO(divan): do we need to report
+			// errors up to the caller?
+			// Ignoring for now, as loop
+			// should keep running.
+			continue
 		}
 	}
-
-	// drain ready channel of any existing tasks
-outer:
-	for {
-		select {
-		case t := <-l.ready:
-			if t != nil {
-				if err := l.processTask(t); err != nil {
-					return err
-				}
-			}
-		default:
-			break outer
-		}
-	}
-
-	close(l.ready)
-
 	return nil
 }

vendor/github.com/status-im/ottoext/loop/looptask/tasks.go

@@ -3,7 +3,7 @@ package looptask
 import (
 	"errors"
 
-	"fknsrs.biz/p/ottoext/loop"
+	"github.com/status-im/ottoext/loop"
 	"github.com/robertkrimen/otto"
 )
 

vendor/github.com/status-im/ottoext/main.go

@@ -1,2 +1,2 @@
 // Package ottoext contains some extensions for the otto JavaScript interpreter.
-package ottoext // import "fknsrs.biz/p/ottoext"
+package ottoext

vendor/github.com/status-im/ottoext/process/process.go

@@ -1,4 +1,4 @@
-package process // import "fknsrs.biz/p/ottoext/process"
+package process
 
 import (
 	"os"

vendor/github.com/status-im/ottoext/promise/promise.go

@@ -1,10 +1,10 @@
-package promise // import "fknsrs.biz/p/ottoext/promise"
+package promise
 
 import (
 	"github.com/robertkrimen/otto"
 
-	"fknsrs.biz/p/ottoext/loop"
-	"fknsrs.biz/p/ottoext/timers"
+	"github.com/status-im/ottoext/loop"
+	"github.com/status-im/ottoext/timers"
 )
 
 func Define(vm *otto.Otto, l *loop.Loop) error {

vendor/github.com/status-im/ottoext/repl/repl.go

@@ -1,16 +1,16 @@
 // Package repl implements an event loop aware REPL (read-eval-print loop)
 // for otto.
-package repl // import "fknsrs.biz/p/ottoext/repl"
+package repl
 
 import (
 	"fmt"
 	"io"
 	"strings"
 
-	"fknsrs.biz/p/ottoext/loop"
-	"fknsrs.biz/p/ottoext/loop/looptask"
 	"github.com/robertkrimen/otto"
 	"github.com/robertkrimen/otto/parser"
+	"github.com/status-im/ottoext/loop"
+	"github.com/status-im/ottoext/loop/looptask"
 	"gopkg.in/readline.v1"
 )
 

vendor/github.com/status-im/ottoext/timers/timers.go

@@ -1,11 +1,11 @@
-package timers // import "fknsrs.biz/p/ottoext/timers"
+package timers
 
 import (
 	"time"
 
 	"github.com/robertkrimen/otto"
 
-	"fknsrs.biz/p/ottoext/loop"
+	"github.com/status-im/ottoext/loop"
 )
 
 var minDelay = map[bool]int64{