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.

Sign up for GitHub

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

net: mapErr causes panic #60029

Closed
levidurfee opened this issue May 7, 2023 · 10 comments
Closed

net: mapErr causes panic #60029

levidurfee opened this issue May 7, 2023 · 10 comments
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@levidurfee
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.20.3 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/levi/.cache/go-build"
GOENV="/home/levi/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/levi/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/levi/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/levi/Projects/git.levi.lol/gopanic/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3232031148=/tmp/go-build -gno-record-gcc-switches"

What did you do?

We are running a service that communicates with another service. I created a repository to maybe help explain.

What did you expect to see?

No issues.

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference

It points to the mapErr function call in lookup.go. It seems like if the mapErr function is passed a nil, then it will panic. I'm not sure if this would be a race issue or what, but it's been difficult to troubleshoot.

  1. The ctx.Done() returns // Done may return nil if this context can never be canceled.
  2. Then ctx.Err() returns nil // If Done is not yet closed, Err returns nil.
  3. Then mapErr causes a panic
vasundhara785 pushed a commit to vasundhara785/go that referenced this issue May 7, 2023
Added to handle panic golang#60029
f3b375a
vasundhara785 added a commit to vasundhara785/go that referenced this issue May 7, 2023
@vasundhara785
Handle panic issue golang#60029
d532f88
@gopherbot
Copy link

Change https://go.dev/cl/493238 mentions this issue: Handle panic issue #60029

@seankhliao
Copy link
Member

Reads from a nil channel block forever, so if ctx.Done() returns nil, then that case will never be called.
I don't see how this can be an error on the net side, it seems more likely to be a bad implementation of a custom context?

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label May 7, 2023
@levidurfee
Copy link
Author

I think there is a custom context, so I'll look into that. Below is the stack trace from the panic in production.

We're only seeing this panic in production. We're unable to reproduce it locally or in any other environment.

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

goroutine 675690 [running]:
net.(*Resolver).lookupIPAddr(0x301a320, {0x2053d08?, 0xc0001fe900}, {0x1b72e75, 0x3}, {0xc0002c56c0, 0xa})
        /home/vsts-agent/actions-runner/_work/_tool/go/1.20.4/x64/src/net/lookup.go:351 +0x904
net.(*Resolver).internetAddrList(0x2053d08?, {0x2053d08?, 0xc0001fe900?}, {0x1b72e75, 0x3}, {0xc0002c56c0?, 0x0?})
        /home/vsts-agent/actions-runner/_work/_tool/go/1.20.4/x64/src/net/ipsock.go:288 +0x519
net.(*Resolver).resolveAddrList(0x304d440?, {0x2053d08, 0xc0001fe900}, {0x1b737aa, 0x4}, {0x1b72e75?, 0x421690?}, {0xc0002c56c0, 0xd}, {0x0, ...})
        /home/vsts-agent/actions-runner/_work/_tool/go/1.20.4/x64/src/net/dial.go:234 +0x41b
net.(*Dialer).DialContext(0x301b2a0, {0x2053d08, 0xc0001fe900}, {0x1b72e75, 0x3}, {0xc0002c56c0, 0xd})
        /home/vsts-agent/actions-runner/_work/_tool/go/1.20.4/x64/src/net/dial.go:422 +0x448
net/http.(*Transport).dial(0x603535?, {0x2053d08?, 0xc0001fe900?}, {0x1b72e75?, 0xc10d5c75a34e9cfe?}, {0xc0002c56c0?, 0x301b320?})
        /home/vsts-agent/actions-runner/_work/_tool/go/1.20.4/x64/src/net/http/transport.go:1185 +0xc5
net/http.(*Transport).dialConn(0xc000145040, {0x2053d08, 0xc0001fe900}, {{}, 0x0, {0xc00004e00f, 0x4}, {0xc0002c56c0, 0xd}, 0x0})
        /home/vsts-agent/actions-runner/_work/_tool/go/1.20.4/x64/src/net/http/transport.go:1614 +0x82c
net/http.(*Transport).dialConnFor(0x705e26?, 0xc0002fbc30)
        /home/vsts-agent/actions-runner/_work/_tool/go/1.20.4/x64/src/net/http/transport.go:1456 +0xb0
created by net/http.(*Transport).queueForDial
        /home/vsts-agent/actions-runner/_work/_tool/go/1.20.4/x64/src/net/http/transport.go:1425 +0x3ea

@seankhliao seankhliao added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels May 7, 2023
@levidurfee
Copy link
Author

Hi @seankhliao,

Since ctx.Done() can return nil, would it be appropriate for net to check for a nil value?

@seankhliao
Copy link
Member

no, as I mentioned before, a nil channel blocks forever, which is the correct behavior here

@seankhliao seankhliao added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels May 8, 2023
@levidurfee
Copy link
Author

I appreciate your patience. Let me ask a different question.

It seems like ctx.Err() could potentially return nil, even if it is unlikely. Would it be appropriate for net to check for a nil value there? Or are you saying that it will never return nil?

@seankhliao
Copy link
Member

https://pkg.go.dev/context#Context

// If Done is not yet closed, Err returns nil.
// If Done is closed, Err returns a non-nil error explaining why:
// Canceled if the context was canceled
// or DeadlineExceeded if the context's deadline passed.
// After Err returns a non-nil error, successive calls to Err return the same error.
Err() error

The case is only triggered when Done is closed, so Err must return a non nil error.

@seankhliao seankhliao added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels May 8, 2023
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@lyouthzzz
Copy link

We have the same problem. ctx is done and then ctx.Error() returns nil. It's a random occurrence in our production environment. we use ip to avoid resolving domain，but this is not a long-term solution. we use custom context,but we can check the context has no problem.

@lyouthzzz
Copy link

@seankhliao @levidurfee Do you have any further plans to solve this problem?

@lyouthzzz lyouthzzz mentioned this issue Nov 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@gopherbot @levidurfee @seankhliao @lyouthzzz