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

Go 1.24 compatibility #227

Closed
ncruces opened this issue Feb 13, 2025 · 10 comments
Closed

Go 1.24 compatibility #227

ncruces opened this issue Feb 13, 2025 · 10 comments
Labels
bug Something isn't working

Comments

@ncruces
Copy link
Owner

ncruces commented Feb 13, 2025

Discussed in #226

Originally posted by olivier-m February 13, 2025
Hi,

I've migrated Readeck (https://codeberg.org/readeck/readeck) from mattn driver to yours and so far, so good. Thanks for your work, I really enjoy not needing CGO anymore :)

Out of curiosity, I compiled the project with Go 1.24.0 and it panics with an "out of bounds memory access". Here's the message:

panic: wasm error: out of bounds memory access
	wasm stack trace:
		sqlite3.wasm.sqlite3BtreeCreateTable(i32,i32,i32) i32
		sqlite3.wasm.sqlite3VdbeExec(i32) i32
		sqlite3.wasm.sqlite3_step(i32) i32
		sqlite3.wasm.sqlite3_exec(i32,i32,i32,i32,i32) i32 [recovered]
	panic: wasm error: out of bounds memory access
	wasm stack trace:
		sqlite3.wasm.sqlite3BtreeCreateTable(i32,i32,i32) i32
		sqlite3.wasm.sqlite3VdbeExec(i32) i32
		sqlite3.wasm.sqlite3_step(i32) i32
		sqlite3.wasm.sqlite3_exec(i32,i32,i32,i32,i32) i32

goroutine 1 [running]:
github.com/doug-martin/goqu/v9.(*TxDatabase).Wrap.func1()
	github.com/doug-martin/goqu/[email protected]/database.go:635 +0xdd
panic({0x159e1e0?, 0xc003353a80?})
	runtime/panic.go:787 +0x132
github.com/ncruces/go-sqlite3.(*sqlite).call(0xc0031f8388, {0x17a463d, 0xc?}, {0xc002297090?, 0xc0002970b8?, 0x4161df?})
	github.com/ncruces/[email protected]/sqlite.go:189 +0x1a7
github.com/ncruces/go-sqlite3.(*Conn).Exec(0xc0009d02a0, {0xc003692800, 0x15a9})
	github.com/ncruces/[email protected]/conn.go:186 +0x106
github.com/ncruces/go-sqlite3/driver.(*conn).ExecContext(0xc000859680, {0x2933d70?, 0x37e99c0?}, {0xc003692800, 0x15a9}, {0x37e99c0?, 0xc001d5fb30?, 0xc000796400?})
	github.com/ncruces/[email protected]/driver/driver.go:405 +0x13f
database/sql.ctxDriverExec({0x2933d70?, 0x37e99c0?}, {0x7aba544a32d8?, 0xc000859680?}, {0x0?, 0x0?}, {0xc003692800?, 0xc0022972c0?}, {0x37e99c0, 0x0, ...})
	database/sql/ctxutil.go:31 +0xd7
database/sql.(*DB).execDC.func2()
	database/sql/sql.go:1713 +0x159
database/sql.withLock({0x292d2c8, 0xc00035f700}, 0xc002297490)
	database/sql/sql.go:3574 +0x71
database/sql.(*DB).execDC(0x7aba5449d2d8?, {0x2933d70, 0x37e99c0}, 0xc00035f700, 0xc003692800?, {0xc003692800, 0x15a9}, {0x0, 0x0, 0x0})
	database/sql/sql.go:1708 +0x216
database/sql.(*Tx).ExecContext(0xc00035f880, {0x2933d70, 0x37e99c0}, {0xc003692800, 0x15a9}, {0x0, 0x0, 0x0})
	database/sql/sql.go:2516 +0xad
github.com/doug-martin/goqu/v9.(*TxDatabase).ExecContext(0xc000a26230, {0x2933d70, 0x37e99c0}, {0xc003692800, 0x15a9}, {0x0, 0x0, 0x0})
	github.com/doug-martin/goqu/[email protected]/database.go:521 +0x8d
github.com/doug-martin/goqu/v9.(*TxDatabase).Exec(...)
	github.com/doug-martin/goqu/[email protected]/database.go:515
codeberg.org/readeck/readeck/internal/db.applyMigrations.func1()
	codeberg.org/readeck/readeck/internal/db/storage.go:179 +0x10f
github.com/doug-martin/goqu/v9.(*TxDatabase).Wrap(0xc000a26000?, 0x2963f20?)
	github.com/doug-martin/goqu/[email protected]/database.go:647 +0x56
codeberg.org/readeck/readeck/internal/db.applyMigrations()
	codeberg.org/readeck/readeck/internal/db/storage.go:169 +0xf6
codeberg.org/readeck/readeck/internal/db.Init(...)
	codeberg.org/readeck/readeck/internal/db/storage.go:140
codeberg.org/readeck/readeck/internal/app.InitApp()
	codeberg.org/readeck/readeck/internal/app/app.go:124 +0x405
codeberg.org/readeck/readeck/internal/app.appPreRun(0xc000022060)
	codeberg.org/readeck/readeck/internal/app/app.go:160 +0xde
codeberg.org/readeck/readeck/internal/app.runServer({0x179b168?, 0x7?}, {0xc000134090, 0x0, 0x0})
	codeberg.org/readeck/readeck/internal/app/serve.go:72 +0xf7
github.com/cristalhq/acmd.(*Runner).Run(0xc0005c0000)
	github.com/cristalhq/[email protected]/acmd.go:300 +0x107
codeberg.org/readeck/readeck/internal/app.Run()
	codeberg.org/readeck/readeck/internal/app/app.go:70 +0xe5
main.main()
	codeberg.org/readeck/readeck/main.go:16 +0x13

This was on first launch during schema creation. On an existing database, it fails (same error) with some specific requests but not all of them.

If you'd like to reproduce, you can compile with:
make setup && make generate && make build after changing the go value to 1.24 in go.mod. I don't know if that's an issue on your side, wazero or somewhere else. Let me know if I can perform more tests.

PS: the migration to WASM was done in this branch: https://codeberg.org/readeck/readeck/pulls/411/files

@ncruces
Copy link
Owner Author

ncruces commented Feb 13, 2025

This looks like tetratelabs/wazero#2375.

I haven't had the time to look into it. I had tested with internal RC builds of 1.24, so I thought SQLite might be OK.

@ncruces ncruces added the bug Something isn't working label Feb 13, 2025
@danp
Copy link

danp commented Feb 13, 2025

For what it's worth I've been using go-sqlite3 over the last couple months with Go tip as it's led up to 1.24 without issues, both on darwin/arm64 and linux/amd64.

For example I have a project I started the other day that's using github.com/ncruces/go-sqlite3 v0.22.0 and github.com/tetratelabs/wazero v1.8.2, built mostly recently with go1.25-215de81513.

@ncruces
Copy link
Owner Author

ncruces commented Feb 13, 2025

Right. Me too. So this has really caught me off guard.

@olivier-m can you tell us more, at a minimum: OS, architecture, etc?

If you can create a smaller reproducer, it might also be helpful.

@danp
Copy link

danp commented Feb 13, 2025

👍 I just tried adding various things I see in sqlite.go here that I don't typically use (memdb, pragmas, etc) to one of my projects and couldn't get it to break at least, on darwin/arm64 with go1.25-43b7e67040.

@ncruces
Copy link
Owner Author

ncruces commented Feb 13, 2025

Also tests seem to pass in CI:
https://github.com/ncruces/go-sqlite3/actions/runs/13319041955

With the exception of wasip1, which might have to do with that port's changes (might be as simple as the way tests must run with wasmtime has changed, nothing to do with wazero or SQLite).

@olivier-m
Copy link

olivier-m commented Feb 14, 2025

Right. Me too. So this has really caught me off guard.

@olivier-m can you tell us more, at a minimum: OS, architecture, etc?

Standard intel 64bit (core i7) on linux:

> uname -srvmo
Linux 6.13.1-2-MANJARO #1 SMP PREEMPT_DYNAMIC Mon, 03 Feb 2025 16:27:16 +0000 x86_64 GNU/Linux

If you can create a smaller reproducer, it might also be helpful.

Here you go:

package main

import (
	"fmt"
	"os"
	"runtime"

	"github.com/ncruces/go-sqlite3/driver"
	_ "github.com/ncruces/go-sqlite3/embed"
)

func main() {
	fmt.Printf("Go version: %s\n", runtime.Version())

	if err := os.Remove("test.db"); err != nil {
		if !os.IsNotExist(err) {
			panic(err)
		}
	}

	db, err := driver.Open("file:test.db?_txlock=immediate&_timefmt=auto")
	if err != nil {
		panic(err)
	}

	if _, err = db.Exec(`
		PRAGMA busy_timeout = 5000;
		PRAGMA foreign_keys = 1;
		PRAGMA journal_mode = WAL;
		PRAGMA mmap_size = 30000000000;
		PRAGMA cache_size = 1000000000;
		PRAGMA secure_delete = 1;
		PRAGMA synchronous = NORMAL;
		PRAGMA temp_store = memory;
	`); err != nil {
		panic(err)
	}

	if _, err = db.Exec(`
		CREATE TABLE migration (
			id      integer  PRIMARY KEY,
			name    text     NOT NULL,
			applied datetime NOT NULL
		);
	`); err != nil {
		panic(err)
	}

	fmt.Println("all good!")
}

Then built with go build -tags "netgo usergo" -trimpath.

Following @danp's hunch, I toyed with the PRAGMA settings and, indeed, there's a culprit.
If you comment PRAGMA secure_delete = 1; (and keep the other statements), it works fine. I'll make some tests and let you know if it fixes the issue on the whole app.

Thanks for the prompt answer :)

Edit: I tried building the whole project without secure_delete but it now fails at the connection.

@ncruces
Copy link
Owner Author

ncruces commented Feb 14, 2025

If it's secure_delete I'm guessing tetratelabs/wazero#2375 is the culprit. See tetratelabs/wazero#2375 (comment) in particular.

I'm betting secure_delete ends up stressing one of the Wasm bulk memory operations that wazero uses memmove for.

It won't be the only usage of it, of course, but it makes sense that it adds more opportunities for memmove to cause havock.

We could try building SQLite without bulk memory but I guess it's simpler to just wait for the wazero fix.

@olivier-m
Copy link

Thanks for your answer, since I don't urgently need any new feature in 1.24, I'll wait for a fix in wazero :)

@ncruces
Copy link
Owner Author

ncruces commented Feb 17, 2025

@olivier-m, can you try tetratelabs/wazero#2378?

This should do it:

go mod edit -replace github.com/tetratelabs/wazero=github.com/anuraaga/wazero@go-124-memmove
go mod tidy
# go test something?

@olivier-m
Copy link

@olivier-m, can you try tetratelabs/wazero#2378?

The app works (manual and e2e tests) and all tests pass. I'd say it's all good :) Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants