Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(760)

Issue 12541052: code review 12541052: runtime: impose stack size limit (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 3 months ago by rsc
Modified:
4 years, 3 months ago
Reviewers:
khr, bradfitz
CC:
khr, r, dvyukov, golang-dev
Visibility:
Public.

Description

runtime: impose stack size limit The goal is to stop only those programs that would keep going and run the machine out of memory, but before they do that. 1 GB on 64-bit, 250 MB on 32-bit. That seems implausibly large, and it can be adjusted. Fixes issue 2556. Fixes issue 4494. Fixes issue 5173.

Patch Set 1 #

Patch Set 2 : diff -r b1c9e72c2ca3 https://code.google.com/p/go/ #

Patch Set 3 : diff -r b1c9e72c2ca3 https://code.google.com/p/go/ #

Total comments: 5

Patch Set 4 : diff -r 249cd3c39e56 https://code.google.com/p/go/ #

Patch Set 5 : diff -r 7c02aea39eae https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+80 lines, -11 lines) Patch
M src/pkg/runtime/crash_test.go View 1 2 3 4 chunks +33 lines, -9 lines 0 comments Download
M src/pkg/runtime/debug/garbage.go View 1 2 3 4 2 chunks +15 lines, -0 lines 0 comments Download
M src/pkg/runtime/panic.c View 1 2 3 1 chunk +3 lines, -1 line 0 comments Download
M src/pkg/runtime/proc.c View 1 2 3 2 chunks +9 lines, -0 lines 0 comments Download
M src/pkg/runtime/runtime.h View 1 2 3 2 chunks +2 lines, -0 lines 0 comments Download
M src/pkg/runtime/stack.c View 1 2 3 3 chunks +18 lines, -1 line 0 comments Download

Messages

Total messages: 14
rsc
Hello khr, r (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
4 years, 3 months ago (2013-08-09 20:01:03 UTC) #1
r
perhaps the number should be less on 32-bit architectures.
4 years, 3 months ago (2013-08-09 20:58:57 UTC) #2
khr
LGTM. https://codereview.appspot.com/12541052/diff/5001/src/pkg/runtime/debug/garbage.go File src/pkg/runtime/debug/garbage.go (right): https://codereview.appspot.com/12541052/diff/5001/src/pkg/runtime/debug/garbage.go#newcode27 src/pkg/runtime/debug/garbage.go:27: func setMaxStack(int) int Why is this in runtime/debug/garbage.go ...
4 years, 3 months ago (2013-08-09 22:12:46 UTC) #3
rsc
On Fri, Aug 9, 2013 at 6:12 PM, <khr@golang.org> wrote: > LGTM. > > https://codereview.appspot.**com/12541052/diff/5001/src/** ...
4 years, 3 months ago (2013-08-09 22:31:22 UTC) #4
dvyukov
Fixes issue 4692.
4 years, 3 months ago (2013-08-10 13:22:55 UTC) #5
dvyukov
crash test please https://codereview.appspot.com/12541052/diff/5001/src/pkg/runtime/debug/garbage.go File src/pkg/runtime/debug/garbage.go (right): https://codereview.appspot.com/12541052/diff/5001/src/pkg/runtime/debug/garbage.go#newcode112 src/pkg/runtime/debug/garbage.go:112: // goroutines that enter an infinite ...
4 years, 3 months ago (2013-08-10 13:37:18 UTC) #6
dvyukov
> https://codereview.appspot.com/12541052/diff/5001/src/pkg/runtime/stack.c#newcode188 > src/pkg/runtime/stack.c:188: uintptr runtime·maxstacksize = 1000000000; // 1 GB > 1<<30 hey, do ...
4 years, 3 months ago (2013-08-10 13:39:37 UTC) #7
rsc
https://codereview.appspot.com/12541052/diff/5001/src/pkg/runtime/stack.c File src/pkg/runtime/stack.c (right): https://codereview.appspot.com/12541052/diff/5001/src/pkg/runtime/stack.c#newcode188 src/pkg/runtime/stack.c:188: uintptr runtime·maxstacksize = 1000000000; // 1 GB On 2013/08/10 ...
4 years, 3 months ago (2013-08-10 13:51:00 UTC) #8
dvyukov
On Sat, Aug 10, 2013 at 5:51 PM, <rsc@golang.org> wrote: > > https://codereview.appspot.**com/12541052/diff/5001/src/** > pkg/runtime/stack.c<https://codereview.appspot.com/12541052/diff/5001/src/pkg/runtime/stack.c> ...
4 years, 3 months ago (2013-08-10 13:57:17 UTC) #9
r
I still think the number should be lowered on 32-bit architectures, which means the constant ...
4 years, 3 months ago (2013-08-10 21:27:00 UTC) #10
rsc
On Sat, Aug 10, 2013 at 5:26 PM, Rob Pike <r@golang.org> wrote: > I still ...
4 years, 3 months ago (2013-08-13 17:34:03 UTC) #11
rsc
I can't reproduce the CgoCrash failures I was seeing. Maybe there was a different bug ...
4 years, 3 months ago (2013-08-16 02:33:42 UTC) #12
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=aeb72c90c10e *** runtime: impose stack size limit The goal is to stop ...
4 years, 3 months ago (2013-08-16 02:34:10 UTC) #13
bradfitz
4 years, 3 months ago (2013-08-16 03:42:24 UTC) #14
Message was sent while issue was closed.
This caused:

run                  nilptr2.go          : incorrect output
runtime: goroutine stack exceeds 250000000-byte limit
fatal error: stack overflow

goroutine 1 [stack split]:
main.func·031()
	/private/tmp/gobuilder/darwin-386-ca87889605e6/go/test/nilptr2.go:76 fp=0xc0f44
main.func·054()
	/private/tmp/gobuilder/darwin-386-ca87889605e6/go/test/nilptr2.go:19 +0x79
fp=0xc0f5c
main.main()
	/private/tmp/gobuilder/darwin-386-ca87889605e6/go/test/nilptr2.go:20 +0xad
fp=0xc0f9c
runtime.main()
	/usr/local/go/src/pkg/runtime/proc.c:209 +0xff fp=0xc0fd0
runtime.goexit()
	/usr/local/go/src/pkg/runtime/proc.c:1373 fp=0xc0fd4

goroutine 2 [runnable]:
runtime.MHeap_Scavenger()
	/usr/local/go/src/pkg/runtime/mheap.c:448
runtime.goexit()
	/usr/local/go/src/pkg/runtime/proc.c:1373
exit status 2

exit status 1
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld c59f3c4