Already cancelled context should not break mysqlConn

[methane]

Description

When already closed context is passed, context-aware APIs like Ping() should return error soon, and not close the connection.

Fixes #858

Checklist

• Code compiles correctly
• Created tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary
• Added myself / the copyright holder to the AUTHORS file

[methane]

@shogo82148 Please look at this.

[shogo82148]

LGTM!

[arvenil]

@methane @shogo82148 It's me again ;)
I just tested it and it doesn't work for me as expected. Now first ping = context deadline exceeded, second ping = no error 😱 and so on.
I've played with this and best results I get with this:

--- a/vendor/github.com/go-sql-driver/mysql/connection.go
+++ b/vendor/github.com/go-sql-driver/mysql/connection.go
@@ -595,10 +595,6 @@ func (mc *mysqlConn) watchCancel(ctx context.Context) error {
 		mc.cleanup()
 		return nil
 	}
-	// When ctx is already cancelled, don't watch it.
-	if err := ctx.Err(); err != nil {
-		return err
-	}
 	// When ctx is not cancellable, don't watch it.
 	if ctx.Done() == nil {
 		return nil
@@ -610,7 +606,7 @@ func (mc *mysqlConn) watchCancel(ctx context.Context) error {
 
 	mc.watching = true
 	mc.watcher <- ctx
-	return nil
+	return ctx.Err()
 }
 
 func (mc *mysqlConn) startWatcher() {

Above works perfectly.

Then however I found out another tiny issue, but maybe it is exactly that - completely different issue. When first Ping doesn't reach context timeout (so, correctly returns nil) then second Ping with timeout that he should definitely reach also returns nil. Another call and it again correctly returns context deadline exceeded. Again happens for me 100% of times.

[methane]

I just tested it and it doesn't work for me as expected. Now first ping = context deadline exceeded, second ping = no error 😱 and so on.

If you are using this code, it's expected behavior.

On first Ping, db.Ping create connection before calling mysqlConn.Ping().
Timed out happend while creating connection, before mysqlConn.Ping() is called.
In this case, mysqlConn.Ping() returns the ctx.Err(), without sending any packet on the connection. It means the connection is in clean state.

On second Ping, db.Ping re-use existing connection.
Since mysqlConn.Ping() is much faster than creating connection, it success without any errors.

[arvenil]

Yes, I think you are right. I was using different code, also with connection pool and different timings. Ok, looks good, thank you very much again and sorry for misleading comment.