use extended error codes and handle closed db connections in lastError

This commit changes the error handling logic so that it respects the
offending result code (instead of only relying on sqlite3_errcode) and
changes the db connection to always report the extended result code
(which eliminates the need to call sqlite3_extended_errcode). These
changes make it possible to correctly and safely handle errors when the
underlying db connection has been closed.

Author: charlievieth

backup.go

@@ -36,7 +36,10 @@ func (destConn *SQLiteConn) Backup(dest string, srcConn *SQLiteConn, src string)
 		runtime.SetFinalizer(bb, (*SQLiteBackup).Finish)
 		return bb, nil
 	}
-	return nil, destConn.lastError()
+	if destConn.db != nil {
+		return nil, destConn.lastError(int(C.sqlite3_extended_errcode(destConn.db)))
+	}
+	return nil, Error{Code: 1, ExtendedCode: 1, err: "backup: destination connection is nil"}
 }
 
 // Step to backs up for one step. Calls the underlying `sqlite3_backup_step`

error.go

@@ -13,13 +13,16 @@ package sqlite3
 #endif
 */
 import "C"
-import "syscall"
+import (
+	"sync"
+	"syscall"
+)
 
 // ErrNo inherit errno.
 type ErrNo int
 
 // ErrNoMask is mask code.
-const ErrNoMask C.int = 0xff
+const ErrNoMask = 0xff
 
 // ErrNoExtended is extended errno.
 type ErrNoExtended int
@@ -85,7 +88,7 @@ func (err Error) Error() string {
 	if err.err != "" {
 		str = err.err
 	} else {
-		str = C.GoString(C.sqlite3_errstr(C.int(err.Code)))
+		str = errorString(int(err.Code))
 	}
 	if err.SystemErrno != 0 {
 		str += ": " + err.SystemErrno.Error()
@@ -148,3 +151,22 @@ const (
 	ErrNoticeRecoverRollback  = ErrNoExtended(ErrNotice | 2<<8)
 	ErrWarningAutoIndex       = ErrNoExtended(ErrWarning | 1<<8)
 )
+
+var errStrCache sync.Map // int => string
+
+// errorString returns the result of sqlite3_errstr for result code rv,
+// which may be cached.
+func errorString(rv int) string {
+	if v, ok := errStrCache.Load(rv); ok {
+		return v.(string)
+	}
+	s := C.GoString(C.sqlite3_errstr(C.int(rv)))
+	// Prevent the cache from growing unbounded by ignoring invalid
+	// error codes.
+	if s != "unknown error" {
+		if v, loaded := errStrCache.LoadOrStore(rv, s); loaded {
+			s = v.(string)
+		}
+	}
+	return s
+}

sqlite3.go

@@ -78,7 +78,6 @@ _sqlite3_bind_blob(sqlite3_stmt *stmt, int n, void *p, int np) {
   return sqlite3_bind_blob(stmt, n, p, np, SQLITE_TRANSIENT);
 }
 
-#include <stdio.h>
 #include <stdint.h>
 
 static int
@@ -141,7 +140,7 @@ _sqlite3_prepare_v2_internal(sqlite3 *db, const char *zSql, int nBytes, sqlite3_
 
 // Our own implementation of ctype.h's isspace (for simplicity and to avoid
 // whatever locale shenanigans are involved with the Libc's isspace).
-static int _sqlite3_isspace(unsigned char c) {
+static inline int _sqlite3_isspace(unsigned char c) {
 	return c == ' ' || c - '\t' < 5;
 }
 
@@ -209,6 +208,7 @@ static int _sqlite3_prepare_v2(sqlite3 *db, const char *zSql, int nBytes, sqlite
 // _sqlite3_exec_no_args executes all of the statements in zSql. None of the
 // statements are allowed to have positional arguments.
 int _sqlite3_exec_no_args(sqlite3 *db, const char *zSql, int nBytes, int64_t *rowid, int64_t *changes) {
+	const char *end = zSql + nBytes;
 	while (*zSql && nBytes > 0) {
 		sqlite3_stmt *stmt;
 		const char *tail;
@@ -226,12 +226,25 @@ int _sqlite3_exec_no_args(sqlite3 *db, const char *zSql, int nBytes, int64_t *ro
 		*changes = sqlite3_changes64(db);
 		*rowid = sqlite3_last_insert_rowid(db);
 
-		sqlite3_finalize(stmt);
+		int rv2 = sqlite3_finalize(stmt);
+		if (rv == SQLITE_OK) {
+			rv = rv2;
+		}
+
 		if (rv != SQLITE_OK && rv != SQLITE_DONE) {
+			// Handle statements that are empty or consist only of a comment.
+			if (rv == SQLITE_MISUSE && sqlite3_errcode(db) == 0) {
+				rv = SQLITE_OK;
+			}
 			return rv;
 		}
 
-		nBytes -= tail - zSql;
+		// Trim leading space to handle statements with trailing whitespace.
+		// This can save us an additional call to sqlite3_prepare_v2.
+		while (tail < end && _sqlite3_isspace(*tail)) {
+			tail++;
+		}
+		nBytes -= (tail - zSql);
 		zSql = tail;
 	}
 	return SQLITE_OK;
@@ -386,6 +399,21 @@ static go_sqlite3_text_column _sqlite3_column_blob(sqlite3_stmt *stmt, int idx)
 	}
 	return r;
 }
+
+typedef struct {
+	const char *msg;
+	int        code;
+} go_sqlite3_db_error;
+
+static go_sqlite3_db_error _sqlite3_db_error(sqlite3 *db) {
+	if (db) {
+		return (go_sqlite3_db_error){
+			.msg = sqlite3_errmsg(db),
+			.code = sqlite3_extended_errcode(db)
+		};
+	}
+	return (go_sqlite3_db_error){ 0 };
+}
 */
 import "C"
 import (
@@ -744,7 +772,7 @@ func (c *SQLiteConn) RegisterCollation(name string, cmp func(string, string) int
 	defer C.free(unsafe.Pointer(cname))
 	rv := C.sqlite3_create_collation(c.db, cname, C.SQLITE_UTF8, handle, (*[0]byte)(unsafe.Pointer(C.compareTrampoline)))
 	if rv != C.SQLITE_OK {
-		return c.lastError()
+		return c.lastError(int(rv))
 	}
 	return nil
 }
@@ -886,7 +914,7 @@ func (c *SQLiteConn) RegisterFunc(name string, impl any, pure bool) error {
 	}
 	rv := sqlite3CreateFunction(c.db, cname, C.int(numArgs), C.int(opts), newHandle(c, &fi), C.callbackTrampoline, nil, nil)
 	if rv != C.SQLITE_OK {
-		return c.lastError()
+		return c.lastError(int(rv))
 	}
 	return nil
 }
@@ -1015,7 +1043,7 @@ func (c *SQLiteConn) RegisterAggregator(name string, impl any, pure bool) error
 	}
 	rv := sqlite3CreateFunction(c.db, cname, C.int(stepNArgs), C.int(opts), newHandle(c, &ai), nil, C.stepTrampoline, C.doneTrampoline)
 	if rv != C.SQLITE_OK {
-		return c.lastError()
+		return c.lastError(int(rv))
 	}
 	return nil
 }
@@ -1027,32 +1055,38 @@ func (c *SQLiteConn) AutoCommit() bool {
 	return int(C.sqlite3_get_autocommit(c.db)) != 0
 }
 
-func (c *SQLiteConn) lastError() error {
-	return lastError(c.db)
+func (c *SQLiteConn) lastError(rv int) error {
+	return lastError(c.db, rv)
 }
 
-// Note: may be called with db == nil
-func lastError(db *C.sqlite3) error {
-	rv := C.sqlite3_errcode(db) // returns SQLITE_NOMEM if db == nil
-	if rv == C.SQLITE_OK {
+func lastError(db *C.sqlite3, rv int) error {
+	if rv == SQLITE_OK {
 		return nil
 	}
-	extrv := C.sqlite3_extended_errcode(db)    // returns SQLITE_NOMEM if db == nil
-	errStr := C.GoString(C.sqlite3_errmsg(db)) // returns "out of memory" if db == nil
+	extrv := rv
+	// Convert the extended result code to a basic result code.
+	rv &= ErrNoMask
 
 	// https://www.sqlite.org/c3ref/system_errno.html
 	// sqlite3_system_errno is only meaningful if the error code was SQLITE_CANTOPEN,
 	// or it was SQLITE_IOERR and the extended code was not SQLITE_IOERR_NOMEM
 	var systemErrno syscall.Errno
-	if rv == C.SQLITE_CANTOPEN || (rv == C.SQLITE_IOERR && extrv != C.SQLITE_IOERR_NOMEM) {
+	if db != nil && (rv == C.SQLITE_CANTOPEN ||
+		(rv == C.SQLITE_IOERR && extrv != C.SQLITE_IOERR_NOMEM)) {
 		systemErrno = syscall.Errno(C.sqlite3_system_errno(db))
 	}
 
+	var msg string
+	if db != nil {
+		msg = C.GoString(C.sqlite3_errmsg(db))
+	} else {
+		msg = errorString(extrv)
+	}
 	return Error{
 		Code:         ErrNo(rv),
 		ExtendedCode: ErrNoExtended(extrv),
 		SystemErrno:  systemErrno,
-		err:          errStr,
+		err:          msg,
 	}
 }
 
@@ -1091,7 +1125,7 @@ func (c *SQLiteConn) execArgs(ctx context.Context, query string, args []driver.N
 		rv := C._sqlite3_prepare_v2(c.db, (*C.char)(unsafe.Pointer(stringData(query))),
 			C.int(len(query)), &s.s, &sz)
 		if rv != C.SQLITE_OK {
-			return nil, c.lastError()
+			return nil, c.lastError(int(rv))
 		}
 		query = strings.TrimSpace(query[sz:])
 
@@ -1143,7 +1177,7 @@ func (c *SQLiteConn) execNoArgsSync(query string) (_ driver.Result, err error) {
 	rv := C._sqlite3_exec_no_args(c.db, (*C.char)(unsafe.Pointer(stringData(query))),
 		C.int(len(query)), &rowid, &changes)
 	if rv != C.SQLITE_OK {
-		err = c.lastError()
+		err = c.lastError(int(rv))
 	}
 	return &SQLiteResult{id: int64(rowid), changes: int64(changes)}, err
 }
@@ -1197,8 +1231,6 @@ func (c *SQLiteConn) Query(query string, args []driver.Value) (driver.Rows, erro
 	return c.query(context.Background(), query, list)
 }
 
-var closedRows = &SQLiteRows{s: &SQLiteStmt{closed: true}}
-
 func (c *SQLiteConn) query(ctx context.Context, query string, args []driver.NamedValue) (driver.Rows, error) {
 	s := SQLiteStmt{c: c, cls: true}
 	p := stringData(query)
@@ -1208,14 +1240,14 @@ func (c *SQLiteConn) query(ctx context.Context, query string, args []driver.Name
 		if rv == C.GO_SQLITE_MULTIPLE_QUERIES {
 			return nil, errors.New("query contains multiple SQL statements")
 		}
-		return nil, c.lastError()
+		return nil, c.lastError(int(rv))
 	}
 
 	// The sqlite3_stmt will be nil if the SQL was valid but did not
 	// contain a query. For now we're supporting this for the sake of
 	// backwards compatibility, but that may change in the future.
 	if s.s == nil {
-		return closedRows, nil
+		return &SQLiteRows{s: &SQLiteStmt{closed: true}}, nil
 	}
 
 	na := int(paramCount)
@@ -1744,7 +1776,7 @@ func (d *SQLiteDriver) Open(dsn string) (driver.Conn, error) {
 	if rv != 0 {
 		// Save off the error _before_ closing the database.
 		// This is safe even if db is nil.
-		err := lastError(db)
+		err := lastError(db, int(rv))
 		if db != nil {
 			C.sqlite3_close_v2(db)
 		}
@@ -1753,13 +1785,18 @@ func (d *SQLiteDriver) Open(dsn string) (driver.Conn, error) {
 	if db == nil {
 		return nil, errors.New("sqlite succeeded without returning a database")
 	}
+	rv = C.sqlite3_extended_result_codes(db, 1)
+	if rv != SQLITE_OK {
+		C.sqlite3_close_v2(db)
+		return nil, lastError(db, int(rv))
+	}
 
 	exec := func(s string) error {
 		cs := C.CString(s)
 		rv := C.sqlite3_exec(db, cs, nil, nil, nil)
 		C.free(unsafe.Pointer(cs))
 		if rv != C.SQLITE_OK {
-			return lastError(db)
+			return lastError(db, int(rv))
 		}
 		return nil
 	}
@@ -2066,7 +2103,7 @@ func (c *SQLiteConn) Close() (err error) {
 	runtime.SetFinalizer(c, nil)
 	rv := C.sqlite3_close_v2(c.db)
 	if rv != C.SQLITE_OK {
-		err = c.lastError()
+		err = lastError(nil, int(rv))
 	}
 	deleteHandles(c)
 	c.db = nil
@@ -2092,7 +2129,7 @@ func (c *SQLiteConn) prepare(ctx context.Context, query string) (driver.Stmt, er
 	var s *C.sqlite3_stmt
 	rv := C._sqlite3_prepare_v2_internal(c.db, (*C.char)(unsafe.Pointer(p)), C.int(len(query)), &s, nil)
 	if rv != C.SQLITE_OK {
-		return nil, c.lastError()
+		return nil, c.lastError(int(rv))
 	}
 	ss := &SQLiteStmt{c: c, s: s}
 	runtime.SetFinalizer(ss, (*SQLiteStmt).Close)
@@ -2160,7 +2197,7 @@ func (c *SQLiteConn) SetFileControlInt(dbName string, op int, arg int) error {
 	cArg := C.int(arg)
 	rv := C.sqlite3_file_control(c.db, cDBName, C.int(op), unsafe.Pointer(&cArg))
 	if rv != C.SQLITE_OK {
-		return c.lastError()
+		return c.lastError(int(rv))
 	}
 	return nil
 }
@@ -2183,7 +2220,7 @@ func (s *SQLiteStmt) Close() error {
 	}
 	rv := C.sqlite3_finalize(stmt)
 	if rv != C.SQLITE_OK {
-		return conn.lastError()
+		return conn.lastError(int(rv))
 	}
 	return nil
 }
@@ -2215,7 +2252,7 @@ func (s *SQLiteStmt) bind(args []driver.NamedValue) error {
 	if s.reset {
 		rv := C.sqlite3_reset(s.s)
 		if rv != C.SQLITE_ROW && rv != C.SQLITE_OK && rv != C.SQLITE_DONE {
-			return s.c.lastError()
+			return s.c.lastError(int(rv))
 		}
 	} else {
 		s.reset = true
@@ -2259,7 +2296,7 @@ func (s *SQLiteStmt) bind(args []driver.NamedValue) error {
 			rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(&b[0])), C.int(len(b)))
 		}
 		if rv != C.SQLITE_OK {
-			return s.c.lastError()
+			return s.c.lastError(int(rv))
 		}
 	}
 	return nil
@@ -2327,7 +2364,7 @@ func (s *SQLiteStmt) bindIndices(args []driver.NamedValue) error {
 				rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(&b[0])), C.int(len(b)))
 			}
 			if rv != C.SQLITE_OK {
-				return s.c.lastError()
+				return s.c.lastError(int(rv))
 			}
 		}
 	}
@@ -2437,7 +2474,7 @@ func (s *SQLiteStmt) execSync(args []driver.NamedValue) (driver.Result, error) {
 	var rowid, changes C.longlong
 	rv := C._sqlite3_step_row_internal(s.s, &rowid, &changes)
 	if rv != C.SQLITE_ROW && rv != C.SQLITE_OK && rv != C.SQLITE_DONE {
-		err := s.c.lastError()
+		err := s.c.lastError(int(rv))
 		C.sqlite3_reset(s.s)
 		C.sqlite3_clear_bindings(s.s)
 		return nil, err
@@ -2473,8 +2510,8 @@ func (rc *SQLiteRows) Close() error {
 	}
 	rv := C.sqlite3_reset(s.s)
 	if rv != C.SQLITE_OK {
-		s.mu.Unlock()
-		return s.c.lastError()
+		rc.s.mu.Unlock()
+		return rc.s.c.lastError(int(rv))
 	}
 	s.mu.Unlock()
 	return nil
@@ -2573,7 +2610,7 @@ func (rc *SQLiteRows) nextSyncLocked(dest []driver.Value) error {
 	if rv != C.SQLITE_ROW {
 		rv = C.sqlite3_reset(rc.s.s)
 		if rv != C.SQLITE_OK {
-			return rc.s.c.lastError()
+			return rc.s.c.lastError(int(rv))
 		}
 		return nil
 	}