Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 3d4e049

Browse filesBrowse files
committed
build: unify setting of ulimit and stack_size
Picking up on @shurcooL's comment in #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.
1 parent f7c5653 commit 3d4e049
Copy full SHA for 3d4e049

File tree

Expand file treeCollapse file tree

2 files changed

+24
-2
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+24
-2
lines changed

‎circle.yml

Copy file name to clipboardExpand all lines: circle.yml
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ test:
2020
- diff -u <(echo -n) <(go list ./compiler/natives/src/...) # All those packages should have // +build js.
2121
- gopherjs install -v net/http # Should build successfully (can't run tests, since only client is supported).
2222
- >
23-
gopherjs test --minify -v --short
23+
ulimit -s 10000 && ulimit -s && gopherjs test --short -v --minify
2424
github.com/gopherjs/gopherjs/tests
2525
github.com/gopherjs/gopherjs/tests/main
2626
github.com/gopherjs/gopherjs/js

‎tool.go

Copy file name to clipboardExpand all lines: tool.go
+23-1Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -750,9 +750,31 @@ func runNode(script string, args []string, dir string, quiet bool) error {
750750
}
751751

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

777+
allArgs = append(allArgs, script)
756778
allArgs = append(allArgs, args...)
757779

758780
node := exec.Command("node", allArgs...)

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.