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

stream: fix panic caused by failing to get a transport for a retry attempt #2958

Merged
merged 1 commit into from
Aug 6, 2019
Merged

stream: fix panic caused by failing to get a transport for a retry attempt #2958

merged 1 commit into from
Aug 6, 2019

Conversation

canguler
Copy link

@canguler canguler commented Aug 6, 2019

To not set new attempt when newAttemptLocked fails to get a transport.

related to #2952 #2954

@canguler canguler requested a review from menghanl August 6, 2019 18:35
gyuho added a commit to gyuho/etcd that referenced this pull request Aug 6, 2019
@gyuho
Copy link
Contributor

gyuho commented Aug 6, 2019

@canguler Sorry, it still panics

--- FAIL: TestCtlV3GetRevokedCRL (5.64s)
    ctl_v3_kv_test.go:68: expected reset connection on put, got read /dev/ptmx: input/output error (expected "OK", got ["

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x9e944e]

goroutine 1 [running]:
go.etcd.io/etcd/vendor/google.golang.org/grpc.(*clientStream).finish(0xc0000b37a0, 0xe657e0, 0xc000178000)
        /go/src/go.etcd.io/etcd/vendor/google.golang.org/grpc/stream.go:824 +0x12e
go.etcd.io/etcd/vendor/google.golang.org/grpc.newClientStream(0xe7c5c0, 0xc0002e2870, 0x14e0520, 0xc000169c00, 0xd4ade3, 0x14, 0xc000260a00, 0x3, 0x4, 0x0, ...)
        /go/src/go.etcd.io/etcd/vendor/google.golang.org/grpc/stream.go:285 +0xbc8
go.etcd.io/etcd/vendor/google.golang.org/grpc.invoke(0xe7c580, 0xc000248c60, 0xd4ade3, 0x14, 0xcf0040, 0xc0002609c0, 0xcdaaa0, 0xc0002f0320, 0xc000169c00, 0xc000260a00, ...)
        /go/src/go.etcd.io/etcd/vendor/google.golang.org/grpc/call.go:66 +0x9c
go.etcd.io/etcd/clientv3.(*Client).unaryClientInterceptor.func1(0xe7c580, 0xc000248c60, 0xd4ade3, 0x14, 0xcf0040, 0xc0002609c0, 0xcdaaa0, 0xc0002f0320, 0xc000169c00, 0xd7f0c8, ...)
        /go/src/go.etcd.io/etcd/clientv3/retry_interceptor.go:57 +0x458
go.etcd.io/etcd/vendor/google.golang.org/grpc.(*ClientConn).Invoke(0xc000169c00, 0xe7c580, 0xc000248c60, 0xd4ade3, 0x14, 0xcf0040, 0xc0002609c0, 0xcdaaa0, 0xc0002f0320, 0x14e09e0, ...)
        /go/src/go.etcd.io/etcd/vendor/google.golang.org/grpc/call.go:35 +0x10a
go.etcd.io/etcd/vendor/google.golang.org/grpc.Invoke(...)
        /go/src/go.etcd.io/etcd/vendor/google.golang.org/grpc/call.go:60
go.etcd.io/etcd/etcdserver/etcdserverpb.(*kVClient).Put(0xc00000e620, 0xe7c580, 0xc000248c60, 0xc0002609c0, 0x14e09e0, 0x3, 0x3, 0xcdd1a0, 0x1, 0xc0002609c0)
        /go/src/go.etcd.io/etcd/etcdserver/etcdserverpb/rpc.pb.go:3477 +0xd3
go.etcd.io/etcd/clientv3.(*retryKVClient).Put(0xc0002f02b0, 0xe7c580, 0xc000248c60, 0xc0002609c0, 0x14e09e0, 0x3, 0x3, 0x8, 0xc0002ee208, 0xc00023b8c8)
        /go/src/go.etcd.io/etcd/clientv3/retry.go:109 +0x7c
go.etcd.io/etcd/clientv3.(*kv).Do(0xc0002e2660, 0xe7c580, 0xc000248c60, 0x2, 0xc0002ee200, 0x1, 0x8, 0x0, 0x0, 0x0, ...)
        /go/src/go.etcd.io/etcd/clientv3/kv.go:156 +0x3b6
go.etcd.io/etcd/clientv3.(*kv).Put(0xc0002e2660, 0xe7c580, 0xc000248c60, 0x7ffe31026e54, 0x1, 0x7ffe31026e56, 0x1, 0x14fdce0, 0x0, 0x0, ...)
        /go/src/go.etcd.io/etcd/clientv3/kv.go:114 +0x116
go.etcd.io/etcd/etcdctl/ctlv3/command.putCommandFunc(0xc00024e280, 0xc00024b030, 0x2, 0x7)
        /go/src/go.etcd.io/etcd/etcdctl/ctlv3/command/put_command.go:71 +0x178
go.etcd.io/etcd/vendor/github.com/spf13/cobra.(*Command).execute(0xc00024e280, 0xc000256dc0, 0x7, 0xa, 0xc00024e280, 0xc000256dc0)
        /go/src/go.etcd.io/etcd/vendor/github.com/spf13/cobra/command.go:766 +0x2ae
go.etcd.io/etcd/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0x14d5200, 0xb, 0xc000054d88, 0xb79701)
        /go/src/go.etcd.io/etcd/vendor/github.com/spf13/cobra/command.go:852 +0x2ec
go.etcd.io/etcd/vendor/github.com/spf13/cobra.(*Command).Execute(...)
        /go/src/go.etcd.io/etcd/vendor/github.com/spf13/cobra/command.go:800
go.etcd.io/etcd/etcdctl/ctlv3.Start()
        /go/src/go.etcd.io/etcd/etcdctl/ctlv3/ctl_nocov.go:25 +0x81
main.main()
        /go/src/go.etcd.io/etcd/etcdctl/main.go:35 +0x77

@canguler
Copy link
Author

canguler commented Aug 6, 2019

Hi @gyuho,

I think your commit is not exactly the same. In finish method, I moved an existing code block whereas you have added a new block. Can you please fix that and retry?

gyuho added a commit to gyuho/etcd that referenced this pull request Aug 6, 2019
@gyuho
Copy link
Contributor

gyuho commented Aug 6, 2019

@canguler Ah, you are right. Let me try again.

@canguler
Copy link
Author

canguler commented Aug 6, 2019

@gyuho,

Any update?

@gyuho
Copy link
Contributor

gyuho commented Aug 6, 2019

@canguler I ran it against this branch, and it seems to fix the panic issue. If anything else comes up, I will open a new issue. Thanks!

gyuho added a commit to etcd-io/etcd that referenced this pull request Aug 6, 2019
@menghanl menghanl changed the title stream: Prevent panic when newAttemptLocked fails to get a transport for the new attempt. stream: not set new attempt when newAttemptLocked fails to get a transport Aug 6, 2019
@menghanl
Copy link
Contributor

menghanl commented Aug 6, 2019

@gyuho This should also fix #2954, can you give it a try? Thanks!

@menghanl menghanl changed the title stream: not set new attempt when newAttemptLocked fails to get a transport stream: fix panic caused by failing to get a transport for a retry attempt Aug 6, 2019
@gyuho
Copy link
Contributor

gyuho commented Aug 6, 2019

@menghanl Yes. If anything else comes up, will create a new issue. Thanks for quick fix!

@canguler canguler merged commit 1f154c6 into grpc:master Aug 6, 2019
@canguler canguler deleted the good-tries branch August 6, 2019 22:36
@hexfusion
Copy link
Contributor

@menghanl any chance we can get this as part of 1.23?

@menghanl
Copy link
Contributor

menghanl commented Aug 7, 2019

@hexfusion Yes, this will go into 1.23.

Also, the change that caused this is not in any release yet, so the past releases should be good.

@menghanl menghanl modified the milestone: 1.23 Release Aug 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Feb 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants