build: unify setting of ulimit and stack_size

myitcv · myitcv · commit 7c769836a2e7 · 2017-08-26T20:43:05.000+01:00
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.
diff --git a/circle.yml b/circle.yml
@@ -18,7 +18,7 @@ test:
     - go tool vet *.go                                                   # Go package in root directory.
     - for d in */; do echo $d; done | grep -v tests/ | grep -v third_party/ | xargs go tool vet # All subdirectories except "tests", "third_party".
     - >
-      gopherjs test --short --minify
+      ulimit -s 10000 && ulimit -s && gopherjs test --short --minify
       github.com/gopherjs/gopherjs/tests
       github.com/gopherjs/gopherjs/tests/main
       github.com/gopherjs/gopherjs/js
diff --git a/tool.go b/tool.go
@@ -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...)
