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

add cycle detection to fmt? #5173

Closed
nictuku opened this issue Mar 31, 2013 · 6 comments
Closed

add cycle detection to fmt? #5173

nictuku opened this issue Mar 31, 2013 · 6 comments

Comments

@nictuku
Copy link
Contributor

nictuku commented Mar 31, 2013

This is a common rookie mistake:

package main

import "fmt"

type A int

func (a A) String() string {
    return fmt.Sprintf("%v", a)
}

func main() {
    var a A
    fmt.Println("this will never print", a)
}

This programs has a cycle calling String(), so it tries to allocate infinite memory and
then gets killed by the kernel. The language is behaving as expected, but debugging this
can be quite tricky. If this happens in a rarely executed section of code, finding what
triggered it is very hard (assuming you don't immediately connect the dots and start
searching for possible sources of unintended recursion). Even getting heap profile is
extremely hard in these circumstances.

Other than hoping that programmers aren't as stupid as me, I wonder if we could make the
program crash with a nicer panic that identifies the bogus recursion, before the kernel
OOM killer sends a SIGKILL.

In Go it's possible[1] to allocate up to 128GB of RAM, so just using a fixed limit for
this wouldn't work.

I guess next time I could tweak the MaxMem constant in malloc.h and try to run the
program again, to get a go-generated panic.

But even better would be Jan Merci's suggestion of having cycle detection in the fmt
package itself. That would probably catch the most common cases.

[1] https://code.google.com/p/go/source/detail?r=a310cb32c278
@bradfitz
Copy link
Contributor

Comment 1:

Cycle detection would be nice only if it could be cheap enough to detect at runtime,
with zero false positives.
You don't want to do a memory check or stack walk in each fmt call, so when do you do it?
I can't think of a good solution.

@dvyukov
Copy link
Member

dvyukov commented Mar 31, 2013

Comment 2:

It can be:
func (a A) String() string {
    a.x++
    if a.x < 1e6 {
      return fmt.Sprintf("%v", a)
    }
    return "foo"
}
So any finite amount of recursion is not a bug in itself.

@bradfitz
Copy link
Contributor

Comment 3:

Agreed.
That's part of "zero false positives".
(I assume you meant a pointer receiver, too.)

@remyoudompheng
Copy link
Contributor

Comment 4:

See also issue #4692.

@robpike
Copy link
Contributor

robpike commented Apr 1, 2013

Comment 5:

It would be nice to catch but is infeasible.

Status changed to Unfortunate.

@rsc
Copy link
Contributor

rsc commented Aug 16, 2013

Comment 6:

This issue was closed by revision 757e0de.

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants