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

encoding/gob: panic in reflect.Value.IsNil when encoding via pointer type alias #12562

Open
jacobsa opened this issue Sep 10, 2015 · 5 comments
Open

encoding/gob: panic in reflect.Value.IsNil when encoding via pointer type alias #12562

jacobsa opened this issue Sep 10, 2015 · 5 comments
Milestone
Unplanned

Comments

@jacobsa
Copy link
Contributor

jacobsa commented Sep 10, 2015

Here is a program containing a type Foo that implements gob.GobEncoder in a silly way: by calling back into gob.Encoder.Encode using a type alias in order to avoid recursing. (The real world use for this is to zero a problematic field and then gob-encode the rest.)

It causes gob.Encoder.Encode to panic in reflect.Value.IsNil:

panic: reflect: call of reflect.Value.IsNil on struct Value [recovered]
    panic: reflect: call of reflect.Value.IsNil on struct Value [recovered]
    panic: reflect: call of reflect.Value.IsNil on struct Value

goroutine 1 [running]:
encoding/gob.catchError(0x1044c3ec, 0x1043a0dc)
    /usr/local/go/src/encoding/gob/error.go:38 +0xe0
encoding/gob.catchError(0x1044c46c, 0x1043a13c)
    /usr/local/go/src/encoding/gob/error.go:38 +0xe0
reflect.Value.IsNil(0x1ad3c0, 0x10438dd0, 0xd9, 0x1042b9c8, 0x1042b9cc, 0xb31c0)
    /usr/local/go/src/reflect/value.go:971 +0x120
encoding/gob.encIndirect(0x1ad3c0, 0x10438dd0, 0xd9, 0x1, 0x0, 0x0, 0x0, 0x52d0)
    /usr/local/go/src/encoding/gob/encode.go:152 +0x60
encoding/gob.(*Encoder).encodeSingle(0x1044c400, 0x1044c420, 0x10438e20, 0x1ad3c0, 0x10438dd0, 0xd9)
    /usr/local/go/src/encoding/gob/encode.go:291 +0x200
encoding/gob.(*Encoder).encode(0x1044c400, 0x1044c420, 0x1ad3c0, 0x10438dd0, 0xd9, 0x10435440)
    /usr/local/go/src/encoding/gob/encode.go:694 +0x280
encoding/gob.(*Encoder).EncodeValue(0x1044c400, 0x18cf80, 0x10438dd0, 0x16, 0x0, 0x0)
    /usr/local/go/src/encoding/gob/encoder.go:247 +0x780
encoding/gob.(*Encoder).Encode(0x1044c400, 0x18cf80, 0x10438dd0, 0x52d0, 0x0, 0x0)
    /usr/local/go/src/encoding/gob/encoder.go:174 +0x80
main.Foo.GobEncode(0x11, 0x52d0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x10438b60)
    /tmp/sandbox454377553/main.go:23 +0x120
main.(*Foo).GobEncode(0x10438b60, 0x1aa4c0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    <autogenerated>:1 +0xc0
encoding/gob.(*Encoder).encodeGobEncoder(0x1044c380, 0x1044c3a0, 0x104350e0, 0x1aa4c0, 0x10438b60, 0x16)
    /usr/local/go/src/encoding/gob/encode.go:469 +0x100
encoding/gob.gobEncodeOpFor.func1(0x104353e0, 0x10435360, 0x1aa4c0, 0x10438b60, 0x16, 0x1)
    /usr/local/go/src/encoding/gob/encode.go:613 +0x260
encoding/gob.(*Encoder).encodeSingle(0x1044c380, 0x1044c3a0, 0x10438db0, 0x1aa4c0, 0x10438b60, 0x16)
    /usr/local/go/src/encoding/gob/encode.go:294 +0x2a0
encoding/gob.(*Encoder).encode(0x1044c380, 0x1044c3a0, 0x1aa4c0, 0x10438b60, 0x16, 0x104350e0)
    /usr/local/go/src/encoding/gob/encode.go:694 +0x280
encoding/gob.(*Encoder).EncodeValue(0x1044c380, 0x1aa4c0, 0x10438b60, 0x16, 0x0, 0x0)
    /usr/local/go/src/encoding/gob/encoder.go:247 +0x780
encoding/gob.(*Encoder).Encode(0x1044c380, 0x1aa4c0, 0x10438b60, 0x1fc26d, 0x0, 0x0)
    /usr/local/go/src/encoding/gob/encoder.go:174 +0x80
main.main()
    /tmp/sandbox454377553/main.go:39 +0x120

If I'm reading the documentation correctly, this program should work as expected (printing Result: 17). But even if it's not intended to work, it probably shouldn't cause a panic.

Note that if you change it so that the alias looks like type Alias Foo, the program works whether you call Encode with Alias(f) or (*Alias)(&f).

Go version:

go version devel +da7e9e4 Thu Sep 10 00:59:04 2015 +0000 darwin/amd64
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Sep 10, 2015
@gopherbot
Copy link

CL https://golang.org/cl/15920 mentions this issue.

@robpike robpike self-assigned this Dec 8, 2015
@robpike robpike modified the milestones: Go1.7, Go1.6 Dec 8, 2015
@rsc
Copy link
Contributor

rsc commented Apr 25, 2016

The program in the report seems fundamentally wrong. It makes a Alias which is *Foo, but then once gob indirects it has a Foo with an Encode method, which is going to put the recursion back. If you 'type Alias Foo', as in http://play.golang.org/p/Lby39nmUE0, then everything works today. There may be cause for a clearer error here but that's all.

@robpike robpike modified the milestones: Unplanned, Go1.7 Apr 25, 2016
@robpike
Copy link
Contributor

robpike commented Apr 25, 2016

Changing to unplanned. I may see if I can stop the panic from happening, but it's not a straightforward change, and since the program cannot work regardless, it's low priority. But leaving open in case inspiration strikes.

@iwdgo
Copy link
Contributor

iwdgo commented Jun 11, 2018

The type interface documented is GobEncoder (and not GobEncode). From the problem description, you want to have the ability to use a type method accessing directly the gob encoder.

The code suggested still crashes online and loops elsewhere because the encoder is redefined. Further *Foo is not a non-interface type as per documentation and the case is not detected.

Below, is the code using GobEncoder/GobDecoder as per documentation which runs using the non-interface type.

type Foo struct {
	I int
}

type Alias *Foo

func (f Foo) GobEncoder() (b []byte, err error) {
	// Encode via the type alias.
	buf := new(bytes.Buffer)
	err = gob.NewEncoder(buf).Encode(Alias(&f))
	b = buf.Bytes()

	return
}

func (f *Foo) GobDecoder(b []byte) (err error) {
	// Decode via the type alias.
	err = gob.NewDecoder(bytes.NewReader(b)).Decode(Alias(f))
	return
}

func main() {
	// Encode.
	f := Foo{17}
	var err error
	var buf []byte
	if buf,err = f.GobEncoder(); err != nil {
		log.Fatalf("Encode: %v", err)
	}

	// Decode.
	f = Foo{}
	if err = f.GobDecoder(buf); err != nil {
		log.Fatalf("Decode: %v", err)
	}

	fmt.Fprintf(os.Stderr, "Result: %d\n", f.I)
}

@valarauca
Copy link

valarauca commented Sep 25, 2018 •
edited

I can recreate this with unary protobuffers middleware in go version go1.11 linux/amd64.

The UnaryServerInterceptor returns an interface{}, which our middleware will reflect off of to extract common metadata keys in cases where erroneous client did not include these values as headers, or to extract additional logging information eagerly before the handler is invoked.

The issue arises as reflect.Value.IsNil panics, but the go compiler itself errors when reflect.Value is compared with == nil.

A sample of code that fails to this is found here. The panic occurs on line 19 when innerValue.IsNil() is called.

Could this possibly be mitigated by modifying this statement to return true? It seems odd to panic on nil when the function is checking for nil, and it knows the value is nil.

@rsc rsc unassigned robpike Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
7 participants
@rsc @jacobsa @ianlancetaylor @robpike @gopherbot @valarauca @iwdgo