Skip to content

Commit

Permalink
build: unify setting of ulimit and stack_size
Browse files Browse the repository at this point in the history
Picking up on @shurcooL's comment in
gopherjs#669 (comment).

We've started randomly seeing std library tests tip over the V8 stack
size limit, causing text/template TestMaxExecDepth to fail randomly
locally and on CI. This had previously been addressed by @neelance in
1f89545; the --stack_size flag was passed to NodeJS which in turn passed
the value onto V8. But per
nodejs/node#14567 (comment) it
was pointed out that the value of ulimit -s must be >= the value of
--stack_size for the --stack_size to make any sort of sense. Hence this
commit also harmonises the setting of ulimit -s during the CI test run
with the value of --stack_size that is passed to NodeJS (which in turn
passes that value onto V8) when running either gopherjs test or gopherjs
run.
  • Loading branch information
myitcv committed Aug 27, 2017
1 parent f7c5653 commit 3d4e049
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
2 changes: 1 addition & 1 deletion circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ test:
- diff -u <(echo -n) <(go list ./compiler/natives/src/...) # All those packages should have // +build js.
- gopherjs install -v net/http # Should build successfully (can't run tests, since only client is supported).
- >
gopherjs test --minify -v --short
ulimit -s 10000 && ulimit -s && gopherjs test --short -v --minify
github.com/gopherjs/gopherjs/tests
github.com/gopherjs/gopherjs/tests/main
github.com/gopherjs/gopherjs/js
Expand Down
24 changes: 23 additions & 1 deletion tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -750,9 +750,31 @@ func runNode(script string, args []string, dir string, quiet bool) error {
}

if runtime.GOOS != "windows" {
allArgs = append(allArgs, "--stack_size=10000", script)
// For non-windows OS environments, we've seen issues with stack space
// limits causeing Go std library tests that are recursion-heavy to fail
// (see https://github.com/gopherjs/gopherjs/issues/661 for more detail).
//
// There are two limits that come into play here, listed in order:
//
// 1. V8 limit (NodeJS effectively wraps V8)
// 2. OS process limit
//
// In order to limit the surface area of the gopherjs command and not
// expose V8 flags/options etc to the caller, we control limit 1 via
// limit 2. That is to say, whatever value is returned by ulimit -s is
// essentially the value that we pass on to NodeJS via the appropriate V8
// flag.
var r syscall.Rlimit
err := syscall.Getrlimit(syscall.RLIMIT_STACK, &r)
if err != nil {
return fmt.Errorf("failed to get stack size limit: %v", err)
}
// rlimit value is in bytes, we need rounded kBytes value per node --v8-options.
stackSize := fmt.Sprintf("--stack_size=%v", r.Cur/1000)
allArgs = append(allArgs, stackSize)
}

allArgs = append(allArgs, script)
allArgs = append(allArgs, args...)

node := exec.Command("node", allArgs...)
Expand Down

0 comments on commit 3d4e049

Please sign in to comment.