Skip to content

Commit b927e41

Browse files
committed
Add support for SQLITE_ENABLE_MULTITHREADED_CHECKS
The build tag `sqlite_enable_multithreaded_checks` enables `SQLITE_ENABLE_MULTITHREADED_CHECKS` in sqlite. sqlite3.c is modified to respect `SQLITE_ENABLE_MULTITHREADED_CHECKS` even on connections that use `SQLITE_OPEN_FULLMUTEX`. Updates tailscale/corp#34473 Signed-off-by: Percy Wegmann <percy@tailscale.com>
1 parent 79b668e commit b927e41

6 files changed

Lines changed: 119 additions & 2 deletions

File tree

.github/workflows/test-sqlite.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ jobs:
2828

2929
- name: With sqlite_enable_tmstmpvfs
3030
run: go test -v -tags sqlite_enable_tmstmpvfs
31-
31+
32+
- name: With sqlite_enable_multithreaded_checks
33+
run: go test -v -tags sqlite_enable_multithreaded_checks
3234
- name: Race
3335
run: go test -v -race ./...
3436

cgosqlite/cgosqlite.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ int tmstmpvfs_enabled=1;
6363
int tmstmpvfs_enabled=0;
6464
#endif
6565
66+
// Enable mutex contention warnings.
67+
#cgo sqlite_enable_multithreaded_checks CFLAGS: -DSQLITE_ENABLE_MULTITHREADED_CHECKS
68+
#ifdef SQLITE_ENABLE_MULTITHREADED_CHECKS
69+
int multithreaded_checks_enabled=1;
70+
#else
71+
int multithreaded_checks_enabled=0;
72+
#endif
73+
6674
#include "cgosqlite.h"
6775
*/
6876
import "C"
@@ -529,3 +537,8 @@ func APIArmorEnabled() bool {
529537
func TimestampVFSEnabled() bool {
530538
return C.tmstmpvfs_enabled == 1
531539
}
540+
541+
// MultithreadedChecksEnabled reports whether or not sqlite was compiled with SQLITE_ENABLE_MULTITHREADED_CHECKS.
542+
func MultithreadedChecksEnabled() bool {
543+
return C.multithreaded_checks_enabled == 1
544+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//go:build !sqlite_enable_multithreaded_checks
2+
3+
package cgosqlite
4+
5+
import (
6+
"testing"
7+
)
8+
9+
func TestMultithreadedChecksDisabled(t *testing.T) {
10+
testMultithreadedChecks(t, false)
11+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//go:build sqlite_enable_multithreaded_checks
2+
3+
package cgosqlite
4+
5+
import (
6+
"testing"
7+
)
8+
9+
func TestMultithreadedChecksEnabled(t *testing.T) {
10+
testMultithreadedChecks(t, true)
11+
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
//
2+
3+
package cgosqlite
4+
5+
import (
6+
"path/filepath"
7+
"runtime"
8+
"sync"
9+
"sync/atomic"
10+
"testing"
11+
12+
"github.com/tailscale/sqlite/sqliteh"
13+
)
14+
15+
// testMultithreadedChecks provides a common function for testing SQLITE_ENABLE_MULTITHREADED_CHECKS.
16+
func testMultithreadedChecks(t *testing.T, wantThreadingWarning bool) {
17+
if wantThreadingWarning && !MultithreadedChecksEnabled() {
18+
t.Fatal("Multithreaded checks are not enabled")
19+
} else if !wantThreadingWarning && MultithreadedChecksEnabled() {
20+
t.Fatal("Multithreaded checks are enabled")
21+
}
22+
23+
var gotMisuseLog atomic.Bool
24+
err := SetLogCallback(func(code sqliteh.Code, msg string) {
25+
if code == sqliteh.SQLITE_MISUSE && msg == "illegal multi-threaded access to database connection" {
26+
gotMisuseLog.Store(true)
27+
}
28+
})
29+
if err != nil {
30+
t.Fatal(err)
31+
}
32+
33+
// Lock this goroutine to a thread (preventing other goroutines from using that thread)
34+
runtime.LockOSThread()
35+
36+
flags := sqliteh.SQLITE_OPEN_READWRITE |
37+
sqliteh.SQLITE_OPEN_CREATE |
38+
sqliteh.SQLITE_OPEN_WAL |
39+
sqliteh.SQLITE_OPEN_URI |
40+
sqliteh.SQLITE_OPEN_FULLMUTEX
41+
// sqliteh.SQLITE_OPEN_FULLMUTEX currently contention checks do not work when opening connection with SQLITE_OPEN_FULLMUTEX
42+
db, err := Open(filepath.Join(t.TempDir(), "test.db"), flags, "")
43+
if err != nil {
44+
t.Fatal(err)
45+
}
46+
defer db.Close()
47+
48+
hitAPI := func() {
49+
for i := 0; i < 1000 && !gotMisuseLog.Load(); i++ {
50+
// Prepare a statement on this thread, mostly ignoring errors.
51+
stmt, _, err := db.Prepare("CREATE TABLE t(c INTEGER PRIMARY KEY)", 0)
52+
if err != nil {
53+
continue
54+
}
55+
if _, err := stmt.Step(nil); err != nil {
56+
continue
57+
}
58+
_ = stmt.Finalize()
59+
}
60+
}
61+
62+
// Hit API on a separate goroutine as well as in this goroutine.
63+
// Because the original goroutine locked the OS thread, this new goroutine
64+
// will execute on a separate thread.
65+
var wg sync.WaitGroup
66+
wg.Add(1)
67+
go func() {
68+
defer wg.Done()
69+
hitAPI()
70+
}()
71+
hitAPI()
72+
wg.Wait()
73+
74+
if wantThreadingWarning && !gotMisuseLog.Load() {
75+
t.Fatal("did not get SQLITE_MISUSE in LogCallback")
76+
}
77+
if !wantThreadingWarning && gotMisuseLog.Load() {
78+
t.Fatal("got SQLITE_MISUSE in LogCallback")
79+
}
80+
}

cgosqlite/sqlite3.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188262,7 +188262,7 @@ static int openDatabase(
188262188262
db = 0;
188263188263
goto opendb_out;
188264188264
}
188265-
if( isThreadsafe==0 ){
188265+
if( isThreadsafe==0 || sqlite3GlobalConfig.bCoreMutex ){
188266188266
sqlite3MutexWarnOnContention(db->mutex);
188267188267
}
188268188268
}

0 commit comments

Comments
 (0)