Fix sqlserver lock

database/sqlserver/sqlserver.go

@@ -198,10 +198,10 @@ func (ss *SQLServer) Lock() error {
 			return err
 		}
 
-		// This will either obtain the lock immediately and return true,
-		// or return false if the lock cannot be acquired immediately.
+		// This will either obtain the lock within 10 seconds and return true,
+		// or return false if the lock cannot be acquired within 10 seconds.
 		// MS Docs: sp_getapplock: https://docs.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-getapplock-transact-sql?view=sql-server-2017
-		query := `EXEC sp_getapplock @Resource = @p1, @LockMode = 'Update', @LockOwner = 'Session', @LockTimeout = 0`
+		query := `EXEC sp_getapplock @Resource = @p1, @LockMode = 'Update', @LockOwner = 'Session', @LockTimeout = 10000`
 
 		var status mssql.ReturnStatus
 		if _, err = ss.conn.ExecContext(context.Background(), query, aid, &status); err == nil && status > -1 {

database/sqlserver/sqlserver_test.go

@@ -8,6 +8,7 @@ import (
 	"log"
 	"runtime"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -104,6 +105,8 @@ func Test(t *testing.T) {
 	t.Run("testMsiTrue", testMsiTrue)
 	t.Run("testOpenWithPasswordAndMSI", testOpenWithPasswordAndMSI)
 	t.Run("testMsiFalse", testMsiFalse)
+	t.Run("testLock", testLock)
+	t.Run("testWithInstanceConcurrent", testWithInstanceConcurrent)
 
 	t.Cleanup(func() {
 		for _, spec := range specs {
@@ -339,3 +342,86 @@ func testMsiFalse(t *testing.T) {
 		}
 	})
 }
+
+func testLock(t *testing.T) {
+	dktesting.ParallelTest(t, specs, func(t *testing.T, c dktest.ContainerInfo) {
+		ip, port, err := c.FirstPort()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		addr := msConnectionString(ip, port)
+		p := &SQLServer{}
+		d, err := p.Open(addr)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		dt.Test(t, d, []byte("SELECT 1"))
+
+		ps := d.(*SQLServer)
+
+		err = ps.Lock()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		err = ps.Unlock()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		err = ps.Lock()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		err = ps.Unlock()
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
+}
+
+func testWithInstanceConcurrent(t *testing.T) {
+	dktesting.ParallelTest(t, specs, func(t *testing.T, c dktest.ContainerInfo) {
+		ip, port, err := c.FirstPort()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// The number of concurrent processes running WithInstance
+		const concurrency = 30
+
+		// We can instantiate a single database handle because it is
+		// actually a connection pool, and so, each of the below go
+		// routines will have a high probability of using a separate
+		// connection, which is something we want to exercise.
+		db, err := sql.Open("sqlserver", msConnectionString(ip, port))
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer func() {
+			if err := db.Close(); err != nil {
+				t.Error(err)
+			}
+		}()
+
+		db.SetMaxIdleConns(concurrency)
+		db.SetMaxOpenConns(concurrency)
+
+		var wg sync.WaitGroup
+		defer wg.Wait()
+
+		wg.Add(concurrency)
+		for i := 0; i < concurrency; i++ {
+			go func(i int) {
+				defer wg.Done()
+				_, err := WithInstance(db, &Config{})
+				if err != nil {
+					t.Errorf("process %d error: %s", i, err)
+				}
+			}(i)
+		}
+	})
+}