addresses finalizer race condition

.github/workflows/go.yml

@@ -18,7 +18,7 @@ jobs:
     - name: Set up Go
       uses: actions/setup-go@v4
       with:
-        go-version: '1.20'
+        go-version: '1.25'
 
     - uses: actions/checkout@v4
 

connection.go

@@ -6,7 +6,6 @@ import "C"
 
 import (
 	"fmt"
-	"runtime"
 	"unsafe"
 )
 
@@ -21,18 +20,15 @@ type Connection struct {
 func OpenConnection(database *Database) (*Connection, error) {
 	conn := &Connection{}
 	conn.database = database
-	runtime.SetFinalizer(conn, func(conn *Connection) {
-		conn.Close()
-	})
 	status := C.lbug_connection_init(&database.cDatabase, &conn.cConnection)
 	if status != C.LbugSuccess {
 		return conn, fmt.Errorf("failed to open connection with status %d", status)
 	}
 	return conn, nil
 }
 
-// Close closes the Connection. Calling this method is optional.
-// The Connection will be closed automatically when it is garbage collected.
+// Close releases the underlying C resources for the connection.
+// MUST be called when done to prevent resource leaks.
 func (conn *Connection) Close() {
 	if conn.isClosed {
 		return
@@ -73,14 +69,11 @@ func (conn *Connection) Query(query string) (*QueryResult, error) {
 	defer C.free(unsafe.Pointer(cQuery))
 	queryResult := &QueryResult{}
 	queryResult.connection = conn
-	runtime.SetFinalizer(queryResult, func(queryResult *QueryResult) {
-		queryResult.Close()
-	})
 	status := C.lbug_connection_query(&conn.cConnection, cQuery, &queryResult.cQueryResult)
 	if status != C.LbugSuccess || !C.lbug_query_result_is_success(&queryResult.cQueryResult) {
 		cErrMsg := C.lbug_query_result_get_error_message(&queryResult.cQueryResult)
 		defer C.lbug_destroy_string(cErrMsg)
-		return queryResult, fmt.Errorf(C.GoString(cErrMsg))
+		return queryResult, fmt.Errorf("%s", C.GoString(cErrMsg))
 	}
 	return queryResult, nil
 }
@@ -96,14 +89,11 @@ func (conn *Connection) Execute(preparedStatement *PreparedStatement, args map[s
 			return queryResult, err
 		}
 	}
-	runtime.SetFinalizer(queryResult, func(queryResult *QueryResult) {
-		queryResult.Close()
-	})
 	status := C.lbug_connection_execute(&conn.cConnection, &preparedStatement.cPreparedStatement, &queryResult.cQueryResult)
 	if status != C.LbugSuccess || !C.lbug_query_result_is_success(&queryResult.cQueryResult) {
 		cErrMsg := C.lbug_query_result_get_error_message(&queryResult.cQueryResult)
 		defer C.lbug_destroy_string(cErrMsg)
-		return queryResult, fmt.Errorf(C.GoString(cErrMsg))
+		return queryResult, fmt.Errorf("%s", C.GoString(cErrMsg))
 	}
 	return queryResult, nil
 }
@@ -134,14 +124,11 @@ func (conn *Connection) Prepare(query string) (*PreparedStatement, error) {
 	defer C.free(unsafe.Pointer(cQuery))
 	preparedStatement := &PreparedStatement{}
 	preparedStatement.connection = conn
-	runtime.SetFinalizer(preparedStatement, func(preparedStatement *PreparedStatement) {
-		preparedStatement.Close()
-	})
 	status := C.lbug_connection_prepare(&conn.cConnection, cQuery, &preparedStatement.cPreparedStatement)
 	if status != C.LbugSuccess || !C.lbug_prepared_statement_is_success(&preparedStatement.cPreparedStatement) {
 		cErrMsg := C.lbug_prepared_statement_get_error_message(&preparedStatement.cPreparedStatement)
 		defer C.lbug_destroy_string(cErrMsg)
-		return preparedStatement, fmt.Errorf(C.GoString(cErrMsg))
+		return preparedStatement, fmt.Errorf("%s", C.GoString(cErrMsg))
 	}
 	return preparedStatement, nil
 }

database.go

@@ -7,7 +7,6 @@ package lbug
 import "C"
 import (
 	"fmt"
-	"runtime"
 	"unsafe"
 )
 
@@ -63,9 +62,6 @@ type Database struct {
 // OpenDatabase opens a Lbug database at the given path with the given system configuration.
 func OpenDatabase(path string, systemConfig SystemConfig) (*Database, error) {
 	db := &Database{}
-	runtime.SetFinalizer(db, func(db *Database) {
-		db.Close()
-	})
 	cPath := C.CString(path)
 	defer C.free(unsafe.Pointer(cPath))
 	cSystemConfig := systemConfig.toC()
@@ -81,8 +77,9 @@ func OpenInMemoryDatabase(systemConfig SystemConfig) (*Database, error) {
 	return OpenDatabase(":memory:", systemConfig)
 }
 
-// Close closes the database. Calling this method is optional.
-// The database will be closed automatically when it is garbage collected.
+// Close releases the underlying C resources for the database.
+// MUST be called when done to prevent resource leaks.
+// Use defer to ensure cleanup: defer db.Close()
 func (db *Database) Close() {
 	if db.isClosed {
 		return

example/go.mod

@@ -1,10 +1,11 @@
 module github.com/LadybugDB/go-ladybug/examples
 
-go 1.20
+go 1.25
 
 replace github.com/LadybugDB/go-ladybug => ../
 
 require github.com/LadybugDB/go-ladybug v0.0.0
 
 require github.com/google/uuid v1.6.0 // indirect
+
 require github.com/shopspring/decimal v1.4.0 // indirect

finalizer_race_test.go

@@ -0,0 +1,203 @@
+package lbug
+
+import (
+	"fmt"
+	"runtime"
+	"sync"
+	"testing"
+)
+
+// The race only manifests when running MULTIPLE tests in batch, not in isolation.
+// That is, if you run the test individually it won't fail.
+// But if you run "go test -v" it will.
+//
+// Key insight: The issue is accumulated GC pressure across test sessions.
+// When test A creates QueryResults and test B runs, the GC may finalize
+// QueryResult A while test B's iteration is still in progress.
+
+// The race occurs when:
+// 1. QueryResult is finalized while FlatTuple.GetValue() is still accessing C memory
+// 2. The GC runs during lbugValueToGoValue() and destroys the parent QueryResult
+// 3. The finalizer calls lbug_query_result_destroy() on memory still in use
+func TestFinalizerRaceCondition(t *testing.T) {
+	// Skip if running with -short (this test can be slow and flaky)
+	if testing.Short() {
+		t.Skip("skipping race condition test in short mode")
+	}
+
+	db, conn := setupTestDatabase(t)
+	defer db.Close()
+	defer conn.Close()
+
+	createTestData(t, conn, 100000)
+
+	const numGoroutines = 20
+	const queriesPerGoroutine = 30
+
+	var wg sync.WaitGroup
+	errChan := make(chan error, numGoroutines*queriesPerGoroutine)
+
+	for g := range numGoroutines {
+		wg.Add(1)
+		go func(goroutineID int) {
+			defer wg.Done()
+
+			for range queriesPerGoroutine {
+				// Query without storing result in a variable that persists
+				// This pattern allows the QueryResult to become "unreachable" quickly
+				if err := runQueryAndIterate(conn); err != nil {
+					errChan <- err
+					return
+				}
+
+				// Force GC to increase likelihood of triggering the race
+				runtime.GC()
+			}
+		}(g)
+	}
+
+	wg.Wait()
+	close(errChan)
+
+	var errors []error
+	for err := range errChan {
+		errors = append(errors, err)
+	}
+
+	if len(errors) > 0 {
+		t.Fatalf("got %d errors during concurrent queries: %v", len(errors), errors[0])
+	}
+}
+
+// setupTestDatabase creates an in-memory database with test schema.
+//
+// Returns the database and connection, which the caller must close.
+func setupTestDatabase(t *testing.T) (*Database, *Connection) {
+	t.Helper()
+
+	db, err := OpenDatabase(":memory:", DefaultSystemConfig())
+	if err != nil {
+		t.Fatalf("failed to open database: %v", err)
+	}
+
+	conn, err := OpenConnection(db)
+	if err != nil {
+		db.Close()
+		t.Fatalf("failed to open connection: %v", err)
+	}
+
+	schemas := []string{
+		`CREATE NODE TABLE Node (
+			id INT64,
+			name STRING,
+			fqn STRING,
+			category STRING,
+			file_path STRING,
+			PRIMARY KEY (id)
+		)`,
+		`CREATE REL TABLE CONNECTS (
+			FROM Node TO Node,
+			label STRING
+		)`,
+	}
+
+	for _, schema := range schemas {
+		result, err := conn.Query(schema)
+		if err != nil {
+			conn.Close()
+			db.Close()
+			t.Fatalf("failed to create schema: %v", err)
+		}
+		result.Close()
+	}
+
+	return db, conn
+}
+
+// createTestData populates the database with synthetic test data.
+// Creates nodes and relationships to simulate a real codebase.
+//
+// numNodes: number of DefinitionNode records to create
+func createTestData(t *testing.T, conn *Connection, numNodes int) {
+	t.Helper()
+
+	const batchSize = 100
+	for i := 0; i < numNodes; i += batchSize {
+		end := min(i + batchSize, numNodes)
+
+		for j := i; j < end; j++ {
+			query := fmt.Sprintf(`
+				CREATE (n:Node {
+					id: %d,
+					name: 'item_%d',
+					fqn: 'src/module%d.item_%d',
+					category: 'entity',
+					file_path: 'src/module%d.ext'
+				})
+			`, j, j, j/10, j, j/10)
+
+			result, err := conn.Query(query)
+			if err != nil {
+				t.Fatalf("failed to insert node %d: %v", j, err)
+			}
+			result.Close()
+		}
+	}
+
+	for i := 0; i < numNodes-3; i++ {
+		for offset := 1; offset <= 3; offset++ {
+			query := fmt.Sprintf(`
+				MATCH (from:Node {id: %d})
+				MATCH (to:Node {id: %d})
+				CREATE (from)-[:CONNECTS {label: 'links'}]->(to)
+			`, i, i+offset)
+
+			result, err := conn.Query(query)
+			if err != nil {
+				continue
+			}
+			result.Close()
+		}
+	}
+}
+
+// runQueryAndIterate executes a query returning OLAP-scale results (15k+ rows).
+// This matches real-world usage patterns where large result sets create GC pressure.
+// The query returns all CONNECTS relationships with multiple columns per row.
+//
+// Returns error if the query or iteration fails.
+func runQueryAndIterate(conn *Connection) error {
+	result, err := conn.Query(`
+		MATCH (source:Node)-[r:CONNECTS]->(target:Node)
+		RETURN source.file_path, source.fqn, source.id, 
+		       target.file_path, target.fqn, target.id,
+		       r.label
+		LIMIT 15000
+	`)
+	if err != nil {
+		return fmt.Errorf("query failed: %w", err)
+	}
+	defer result.Close()
+
+	rowCount := 0
+	for result.HasNext() {
+		row, err := result.Next()
+		if err != nil {
+			return fmt.Errorf("Next() failed at row %d: %w", rowCount, err)
+		}
+
+		// Access all 7 columns - each GetValue enters a race
+		for col := range uint64(7) {
+			_, err = row.GetValue(col)
+			if err != nil {
+				row.Close()
+				return fmt.Errorf("GetValue(%d) failed at row %d: %w", col, rowCount, err)
+			}
+		}
+		row.Close()
+
+		rowCount++
+	}
+
+	return nil
+}

flat_tuple.go

@@ -12,8 +12,8 @@ type FlatTuple struct {
 	isClosed    bool
 }
 
-// Close closes the FlatTuple. Calling this method is optional.
-// The FlatTuple will be closed automatically when it is garbage collected.
+// Close releases the underlying C resources for the FlatTuple.
+// MUST be called when done to prevent resource leaks.
 func (tuple *FlatTuple) Close() {
 	if tuple.isClosed {
 		return

go.mod

@@ -1,6 +1,6 @@
 module github.com/LadybugDB/go-ladybug
 
-go 1.20
+go 1.25
 
 require github.com/google/uuid v1.6.0
 

prepared_statement.go

@@ -13,8 +13,8 @@ type PreparedStatement struct {
 	isClosed           bool
 }
 
-// Close closes the PreparedStatement. Calling this method is optional.
-// The PreparedStatement will be closed automatically when it is garbage collected.
+// Close releases the underlying C resources for the PreparedStatement.
+// MUST be called when done to prevent resource leaks.
 func (stmt *PreparedStatement) Close() {
 	if stmt.isClosed {
 		return