Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

runtime: allow recursion depth limit #4692

Closed
huandu opened this issue Jan 23, 2013 · 9 comments
Closed

runtime: allow recursion depth limit #4692

huandu opened this issue Jan 23, 2013 · 9 comments
Labels
FrozenDueToAge
Milestone
Go1.2

Comments

@huandu
Copy link
Contributor

huandu commented Jan 23, 2013 

Per discussion in #4494 and
https://groups.google.com/forum/?fromgroups=#!topic/golang-nuts/hfEHWJKnHls, it's hard
to observe infinite recursion call in go. It's quite painful to find out root cause in a
large code base at least for me.

My suggestion is to introduce an env variable called GOMAXRECURSIONS to set a hard limit
on depth of recursion call. Once the limit is hit, panic the running goroutine. User
should be able to change its value at runtime, just like GOMAXPROCS.

The default limit can be a very large number, e.g. max int64, or just reuse current
strategy (no limit) to avoid any potential performance regression.

This variable should be designed for DEBUG purpose. Users can choose to set it in debug
version and unset it in product version. Or just set it when product version sucks up
abnormal cpu and memory without any clue. So, even if setting this variable will cause a
program 50% slower, it's still acceptable to me.

FYI: I don't think there would any scenario requires unlimit recursion call depth. It
would be great if go complier or vet or runtime could DETECT such infinite recursion and
show error/panic goroutine. Such detection logic would be quite risky (too smart,
maybe), so I wouldn't suggest to implement it. But I guess this is what I truly want...
Just FYI...
@rsc
Copy link
Contributor

rsc commented Jan 29, 2013

Comment 1:

Labels changed: added priority-later, go1.1maybe, removed priority-triage, go1.1.

Status changed to Thinking.

@robpike
Copy link
Contributor

robpike commented Mar 7, 2013

Comment 2:

Labels changed: removed go1.1maybe.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 3:

I propose that we introduce in package runtime/debug:
// SetMaxStack sets the maximum amount of memory that
// can be used by a single goroutine stack.
// If any goroutine reaches this limit, the program crashes.
// Reducing the maximum only limits future stack growth.
// SetMaxStack returns the previous setting.
// The initial setting is 1 GB.
//
// SetMaxStack is useful mainly for limiting the damage done by
// goroutines that enter an infinite recursion.
func SetMaxStack(bytes int64) int64

Labels changed: added go1.2.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 4:

Issue #2556 has been merged into this issue.

@randall77
Copy link
Contributor

Comment 5:

Should it crash the program, or just panic the goroutine?

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 6:

I believe it should crash the program.
To date, ordinary calls have not been something that could panic.
Introducing unexpected panics could make programs newly flaky.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 7:

Labels changed: added feature.

@dvyukov
Copy link
Member

dvyukov commented Aug 10, 2013

Comment 8:

https://golang.org/cl/12541052/

Owner changed to @rsc.

Status changed to Started.

@dvyukov
Copy link
Member

dvyukov commented Aug 16, 2013

Comment 9:

*** Submitted as https://code.google.com/p/go/source/detail?r=aeb72c90c10e ***
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.
R=khr, r, dvyukov
CC=golang-dev
https://golang.org/cl/12541052

Status changed to Fixed.

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
@rsc rsc removed their assignment Jun 22, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.2
Development

No branches or pull requests
6 participants
@rsc @huandu @dvyukov @robpike @randall77 @gopherbot