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

Not implemented panic in go 1.16 #639

Closed
liorblob opened this issue Feb 17, 2021 · 12 comments
Closed

Not implemented panic in go 1.16 #639

liorblob opened this issue Feb 17, 2021 · 12 comments

Comments

@liorblob
Copy link

liorblob commented Feb 17, 2021

Got the following error when I tried to use Ping after upgrading go to 1.16:

panic: Not implemented [recovered]
	panic: Not implemented
goroutine 8 [running]:
testing.tRunner.func1.2(0xb2b8a0, 0xcd5480)
	/usr/local/go/src/testing/testing.go:1144 +0x332
testing.tRunner.func1(0xc000102a80)
	/usr/local/go/src/testing/testing.go:1147 +0x4b6
panic(0xb2b8a0, 0xcd5480)
	/usr/local/go/src/runtime/panic.go:965 +0x1b9
github.com/denisenkom/go-mssqldb.timeoutConn.SetWriteDeadline(...)
	/builds/gopath/pkg/mod/github.com/denisenkom/go-mssqldb@v0.0.0-20190820223206-44cdfe8d8ba9/net.go:102
crypto/tls.(*Conn).SetWriteDeadline(...)
	/usr/local/go/src/crypto/tls/conn.go:150
crypto/tls.(*Conn).closeNotify(0xc0000be000, 0x0, 0x0)
	/usr/local/go/src/crypto/tls/conn.go:1360 +0xfd
crypto/tls.(*Conn).Close(0xc0000be000, 0x0, 0x0)
	/usr/local/go/src/crypto/tls/conn.go:1330 +0xd7
github.com/denisenkom/go-mssqldb.(*Conn).Close(0xc000aae000, 0x0, 0x0)
	/builds/gopath/pkg/mod/github.com/denisenkom/go-mssqldb@v0.0.0-20190820223206-44cdfe8d8ba9/mssql.go:361 +0x3a
database/sql.(*driverConn).finalClose.func2()
	/usr/local/go/src/database/sql/sql.go:592 +0x49
database/sql.withLock(0xce7670, 0xc0000e8000, 0xc0006f5918)
	/usr/local/go/src/database/sql/sql.go:3294 +0x69
database/sql.(*driverConn).finalClose(0xc0000e8000, 0x0, 0xc0006f5998)
	/usr/local/go/src/database/sql/sql.go:590 +0x13c
database/sql.(*driverConn).Close(0xc0000e8000, 0xc0000e8000, 0x0)
	/usr/local/go/src/database/sql/sql.go:571 +0x135
database/sql.(*DB).putConn(0xc0003a4f70, 0xc0000e8000, 0x0, 0x0, 0xce7601)
	/usr/local/go/src/database/sql/sql.go:1398 +0x2f7
database/sql.(*driverConn).releaseConn(...)
	/usr/local/go/src/database/sql/sql.go:475
database/sql.(*DB).pingDC(0xc0003a4f70, 0xcece50, 0xc0000460b0, 0xc0000e8000, 0xc0006f5b60, 0x0, 0x0)
	/usr/local/go/src/database/sql/sql.go:788 +0x84
database/sql.(*DB).PingContext(0xc0003a4f70, 0xcece50, 0xc0000460b0, 0x0, 0x0)
	/usr/local/go/src/database/sql/sql.go:811 +0x185
database/sql.(*DB).Ping(...)
	/usr/local/go/src/database/sql/sql.go:817

Looking at the 1.16 release notes, this change might be the cause:
https://golang.org/doc/go1.16#crypto/tls
A default write deadline is now set in Conn.Close before sending the "close notify" alert, in order to prevent blocking indefinitely.

func (c timeoutConn) SetWriteDeadline(t time.Time) error {

@jpvillaseca
Copy link

It also happened to me in Go 1.16, but when reverting to Go 1.15.6 the issue did not occur.

panic: Not implemented

goroutine 36 [running]:
github.com/denisenkom/go-mssqldb.passthroughConn.SetWriteDeadline(...)
	/go/pkg/mod/github.com/denisenkom/go-mssqldb@v0.9.0/net.go:167
crypto/tls.(*Conn).SetWriteDeadline(...)
	/usr/local/go/src/crypto/tls/conn.go:150
crypto/tls.(*Conn).closeNotify(0xc000094000, 0x0, 0x0)
	/usr/local/go/src/crypto/tls/conn.go:1360 +0xfd
crypto/tls.(*Conn).Close(0xc000094000, 0x0, 0x0)
	/usr/local/go/src/crypto/tls/conn.go:1330 +0xd7
github.com/denisenkom/go-mssqldb.(*Conn).Close(0xc00009e500, 0x0, 0x0)
	/go/pkg/mod/github.com/denisenkom/go-mssqldb@v0.9.0/mssql.go:361 +0x3a
database/sql.(*driverConn).finalClose.func2()
	/usr/local/go/src/database/sql/sql.go:592 +0x49
database/sql.withLock(0x116b6d0, 0xc0004a8120, 0xc000058670)
	/usr/local/go/src/database/sql/sql.go:3294 +0x69
database/sql.(*driverConn).finalClose(0xc0004a8120, 0xc000480000, 0xc000438808)
	/usr/local/go/src/database/sql/sql.go:590 +0x13c
database/sql.(*driverConn).Close(0xc0004a8120, 0xc00082c458, 0x1)
	/usr/local/go/src/database/sql/sql.go:571 +0x135
database/sql.(*DB).connectionCleaner(0xc0003e8410, 0xd18c2e2800)
	/usr/local/go/src/database/sql/sql.go:1022 +0x6d
created by database/sql.(*DB).startCleanerLocked
	/usr/local/go/src/database/sql/sql.go:992 +0xb7

Thank you @liorblob mentioning that it was related to 1.16, it helped me to prevent the issue in production.

@spetr
Copy link

spetr commented Feb 19, 2021

You can test fixed version with this go.mod modification (add row "replace"):

module .....
go 1.16

require (
  github.com/denisenkom/go-mssqldb v0.9.0
  ....
)

replace github.com/denisenkom/go-mssqldb => github.com/spetr/go-mssqldb v0.9.1-0.20210219202709-e432f8ee7c66

@Woody1193
Copy link

I had a similar issue when calling Close:

panic: Not implemented

goroutine 73 [running]:
github.com/denisenkom/go-mssqldb.passthroughConn.SetWriteDeadline(...)
        C:/Users/ryanw/Documents/go/pkg/mod/github.com/denisenkom/go-mssqldb@v0.0.0-20201105052611-0f236ab67f08/net.go:167
crypto/tls.(*Conn).SetWriteDeadline(...)
        C:/Program Files/Go/src/crypto/tls/conn.go:150
crypto/tls.(*Conn).closeNotify(0xc000da1880, 0x0, 0x0)
        C:/Program Files/Go/src/crypto/tls/conn.go:1360 +0x10d
crypto/tls.(*Conn).Close(0xc000da1880, 0x0, 0x0)
        C:/Program Files/Go/src/crypto/tls/conn.go:1330 +0xe5
github.com/denisenkom/go-mssqldb.(*Conn).Close(0xc000b22080, 0x0, 0x0)
        C:/Users/ryanw/Documents/go/pkg/mod/github.com/denisenkom/go-mssqldb@v0.0.0-20201105052611-0f236ab67f08/mssql.go:365 +0x42
database/sql.(*driverConn).finalClose.func2()
        C:/Program Files/Go/src/database/sql/sql.go:592 +0x50
database/sql.withLock(0x18e0c48, 0xc0000922d0, 0xc0005b16b0)
        C:/Program Files/Go/src/database/sql/sql.go:3294 +0x70
database/sql.(*driverConn).finalClose(0xc0000922d0, 0xc0005b1758, 0xf8a459)
        C:/Program Files/Go/src/database/sql/sql.go:590 +0x145
database/sql.(*driverConn).Close(0xc0000922d0, 0xc0000922d0, 0x0)
        C:/Program Files/Go/src/database/sql/sql.go:571 +0x135
database/sql.(*DB).putConn(0xc000bb6000, 0xc0000922d0, 0x0, 0x0, 0x1)
        C:/Program Files/Go/src/database/sql/sql.go:1398 +0x2f7
database/sql.(*driverConn).releaseConn(...)
        C:/Program Files/Go/src/database/sql/sql.go:475
database/sql.(*Conn).close(0xc000940180, 0x0, 0x0, 0x0, 0x0)
        C:/Program Files/Go/src/database/sql/sql.go:1998 +0xb9
database/sql.(*Conn).Close(...)
        C:/Program Files/Go/src/database/sql/sql.go:2010

@uvw
Copy link

uvw commented Mar 17, 2021

I've been running this PR #642 in non-production environments for a week now and haven't found any issues so far.

Not sure if the PR fixes the issue correctly or if something is missing. A review from the authors/maintainers would be great.

To switch to this fix, add replace github.com/denisenkom/go-mssqldb => github.com/robinknaapen/go-mssqldb v0.9.1-0.20210310163057-81bf11eac854 to go.mod.

@lw-bmc
Copy link

lw-bmc commented Apr 1, 2021

I've reported this in github.com/jmoiron/sqlx. I've included a reproducible scenario. When I build an application with Go 1.13, all works fine, but same app, built using 1.16.2 breaks.

jmoiron/sqlx#719 (comment)

@jason-swissre
Copy link

Hi

Has there been any movement on this? My team is hitting this problem now too.

@atc0005
Copy link

atc0005 commented Apr 1, 2021

@dimdin , @kardianos - do either of you have review/merge privileges to this repo?

CC: @denisenkom

@dimdin
Copy link
Collaborator

dimdin commented Apr 1, 2021

@dimdin , @kardianos - do either of you have review/merge privileges to this repo?

CC: @denisenkom
I' ll try to have a fix for that. It must work with both new and old versions that don't have deadlines.

@Steiniche
Copy link

Steiniche commented Apr 6, 2021

Just tested this with golang 1.16 and the fix suggested by @uvw works.

I can see the concern that it must be backward compatible, but AFAIK this is not a problem as older go versions did not call SetWriteDeadline().

Hope someone with merge rights have time to look at the PR soon so we don't have to force a replace in go.mod to the specific commit in the PR.

@denisenkom
Copy link
Owner

PR is merged, please check if there are no issues at this point

BCook98 added a commit to SafetyCulture/safetyculture-exporter that referenced this issue Apr 20, 2021
A patch has been released to fix panics on Go 1.16. denisenkom/go-mssqldb#639

This was imported but not released in go-gorm/sqlserver#27. Pulling in the prerelease commit so that we can release a patch to our customers.
BCook98 added a commit to SafetyCulture/safetyculture-exporter that referenced this issue Apr 20, 2021
A patch has been released to fix panics on Go 1.16. denisenkom/go-mssqldb#639

This was imported but not released in go-gorm/sqlserver#27. Pulling in the prerelease commit so that we can release a patch to our customers.
@atc0005
Copy link

atc0005 commented Apr 27, 2021

PR is merged, please check if there are no issues at this point

FWIW, the new release is working for me.

@kardianos
Copy link
Collaborator

Looks like this is fixed and a release has been issued. Closing.

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