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

Nil Pointer in MarshalTo/MarshalToSizedBuffer #651

Open
un000 opened this issue Dec 17, 2019 · 4 comments
Open

Nil Pointer in MarshalTo/MarshalToSizedBuffer #651

un000 opened this issue Dec 17, 2019 · 4 comments

Comments

@un000
Copy link

un000 commented Dec 17, 2019

Got a grpc handler:

func (h *hello) Echo(ctx context.Context, req *pb.Payload) (*pb.Payload, error) {
	var res *pb.Payload    // <<<< typed nil or can be not typed
	return res, nil
}

With default grpc-library there is no panic, protobuf 1.3.2 returns to a client an error:

	ErrNil = errors.New("proto: Marshal called with nil")

But with gogofaster grpc server crashes with a panic:

[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x14964a6]

goroutine 51 [running]:
github.com/myorg/hello/example/pb.(*Payload).MarshalToSizedBuffer(0x0, 0x1ab1d58, 0x0, 0x0, 0x100bd73, 0x15879e0, 0x15fca60)
        /Users/un0/go/src/github.com/myorg/hello/example/pb/hello.pb.go:192 +0x26
github.com/myorg/hello/example/pb.(*Payload).Marshal(0x0, 0x15fca60, 0x0, 0x4a000a0, 0x0, 0x15fca01)
        /Users/un0/go/src/github.com/myorg/hello/example/pb/hello.pb.go:175 +0xc3
google.golang.org/grpc/encoding/proto.codec.Marshal(0x15fca60, 0x0, 0xc000211070, 0x0, 0x100a9bb, 0xc000012000, 0x158b820)
        /Users/un0/go/pkg/mod/google.golang.org/grpc@v1.24.0/encoding/proto/proto.go:70 +0x194
google.golang.org/grpc.encode(0x4a00028, 0x1ab1d58, 0x15fca60, 0x0, 0x1ab1d58, 0x100a9bb, 0xc000012000, 0x1587f60, 0x1573ca0)
        /Users/un0/go/pkg/mod/google.golang.org/grpc@v1.24.0/rpc_util.go:543 +0x52
google.golang.org/grpc.(*Server).sendResponse(0xc0000eac60, 0x16f6840, 0xc000001e00, 0xc0001ba100, 0x15fca60, 0x0, 0x0, 0x0, 0xc0001a807c, 0x0, ...)
        /Users/un0/go/pkg/mod/google.golang.org/grpc@v1.24.0/server.go:833 +0x89
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0000eac60, 0x16f6840, 0xc000001e00, 0xc0001ba100, 0xc00008fb90, 0x1a85230, 0x0, 0x0, 0x0)
        /Users/un0/go/pkg/mod/google.golang.org/grpc@v1.24.0/server.go:1030 +0x551
google.golang.org/grpc.(*Server).handleStream(0xc0000eac60, 0x16f6840, 0xc000001e00, 0xc0001ba100, 0x0)
        /Users/un0/go/pkg/mod/google.golang.org/grpc@v1.24.0/server.go:1275 +0xd97
google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc00002a5c0, 0xc0000eac60, 0x16f6840, 0xc000001e00, 0xc0001ba100)
        /Users/un0/go/pkg/mod/google.golang.org/grpc@v1.24.0/server.go:710 +0xbb
created by google.golang.org/grpc.(*Server).serveStreams.func1
        /Users/un0/go/pkg/mod/google.golang.org/grpc@v1.24.0/server.go:708 +0xa1

Generated code:

func (m *Payload) MarshalToSizedBuffer(dAtA []byte) (int, error) {
        // if m == nil { return nil, ErrNil }

	i := len(dAtA)    // if len(dAtA) == 0 we must exit with an error, .Size method makes nil check
	_ = i
	var l int
	_ = l

	if m.Number != 0 {      // <<<< nil-pointer panic  m

There should be the check for a backward compatibility.
The entire application must not to be crashed in the runtime, it must return an error.
The right logic was described here grpc/grpc-go#532 (comment)

References #451 (comment)

@un000
Copy link
Author

un000 commented Dec 17, 2019

@awalterschulze @jmarais what do you think about that? Should I commit this change?

@alexshtin
Copy link

I am having the same problem here. Basically every time when error is nil, payload must be not nil which is really annoying. Added checks to handlers for now, but this is very error prone.

Can we fix it? I don't want to switch to standard gRCP library due to number of reasons.

@kevindiu
Copy link

kevindiu commented Aug 5, 2021

Is there any update on this issue?
This panic still occur in my environment and causing the application down...

@jansoneo
Copy link

gogofaster is non-thread-safe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants