Race condition between Go's GC finalizer and active query result iteration

[jkumz]

There is a race condition between Go’s Garbage Collector and Cgo execution. The library currently suffers from intermittent SIGSEGV crashes (as seen in the stack trace) because Go finalizers are being triggered for Parent objects while Child objects are still accessing their memory via Cgo.

The trace shows Goroutine 35 (Go's internal Finalizer goroutine) calling lbug_query_result_destroy while Goroutine 133 is simultaneously executing lbug_flat_tuple_get_value. This results in a Use-After-Free crash because the QueryResult's memory buffer was freed by the finalizer while the FlatTuple was still reading from it

Trunacted stack trace:

goroutine 35 gp=0x14000180540 m=8 [syscall]:
runtime.cgocall(0x10537c1b8, 0x1400006e518)
github.com/LadybugDB/go-ladybug._Cfunc_lbug_query_result_destroy(0x14000825540)
github.com/LadybugDB/go-ladybug.(*QueryResult).Close.func1(...)
        /go-ladybug@v0.13.1/query_result.go:39

goroutine 133 gp=0x14000181880 m=nil [runnable]:
github.com/LadybugDB/go-ladybug.lbugValueToGoValue({0x152a23d38, ...})
        /go-ladybug@v0.13.1/value_helper.go:261
github.com/LadybugDB/go-ladybug.(*FlatTuple).GetValue(0x14000c10200, 0x1)
        /go-ladybug@v0.13.1/flat_tuple.go:77

Go's GC marks a QueryResult as unreachable as soon as the Go code no longer references it—even if a FlatTuple derived from it is still inside a C function. Go is unaware that the C-level FlatTuple depends on the QueryResult's memory.

Currently, finalizers are registered before checking the C return status (e.g., in QueryResult.Next() or OpenConnection()). If the C call fails the GC eventually collects this failed object calling the destroy function on a null or uninitialised handle and crashing.

[jkumz]

Should note I am encountering this quite often, would estimate 30% of the time my unit tests run I get this. I'll try fix when I get some free time.

[adsharma]

Are you thinking about something like this?

https://gist.github.com/adsharma/634c173c32e6ab2ca6a03b5d880231b6

[jkumz]

Are you thinking about something like this?

https://gist.github.com/adsharma/634c173c32e6ab2ca6a03b5d880231b6

Not sure if using a mutex is the best approach here because every time you iterate Next to next row you would be locking and unlocking and this is in the hot path of iterating over results. Was thinking more using runtime.KeepAlive after C calls to make sure the C memory doesn't get released until they successfully return. And then set finalizer after status check.

Something like this:

In flat_tuple.go:

func (tuple *FlatTuple) GetValue(index uint64) (any, error) {
    var cValue C.lbug_value
    
    status := C.lbug_flat_tuple_get_value(&tuple.cFlatTuple, C.uint64_t(index), &cValue)
    
    if status != C.LbugSuccess {
        return nil, fmt.Errorf("failed to get value with status: %d", status)
    }
    
	val, err := lbugValueToGoValue(cValue)
	// release when all C calls are done
    runtime.KeepAlive(tuple) 
	if err != nil {
		return nil, err
	}
	return val, nil
}

In query_result.go:

// HasNext returns true if there is at least one more tuple in the result set.
func (queryResult *QueryResult) HasNext() bool {
    res := bool(C.lbug_query_result_has_next(&queryResult.cQueryResult))
    
    // Prevent queryResult from being freed while checking
    runtime.KeepAlive(queryResult)
    return res
}

// Next returns the next tuple in the result set.
func (queryResult *QueryResult) Next() (*FlatTuple, error) {
    tuple := &FlatTuple{queryResult: queryResult}
    
    status := C.lbug_query_result_get_next(&queryResult.cQueryResult, &tuple.cFlatTuple)
    
    // Prevent the result buffer from being freed while C is filling the tuple
    runtime.KeepAlive(queryResult)

    if status != C.LbugSuccess {
        // handle might be invalid, trying to destroy here may crash
        return nil, fmt.Errorf("failed to get next tuple")
    }

    runtime.SetFinalizer(tuple, func(tuple *FlatTuple) {
		tuple.Close()
	})
    return tuple, nil
}

A grep search shows it should probably be applied to more locations to prevent UAF:

go-ladybug % rg SetFinalizer -C 10
query_result.go
77-}
78-
79-// HasNext returns true if there is at least one more tuple in the result set.
80-func (queryResult *QueryResult) HasNext() bool {
81-     return bool(C.lbug_query_result_has_next(&queryResult.cQueryResult))
82-}
83-
84-// Next returns the next tuple in the result set.
85-func (queryResult *QueryResult) Next() (*FlatTuple, error) {
86-     tuple := &FlatTuple{}
87:     runtime.SetFinalizer(tuple, func(tuple *FlatTuple) {
88-             tuple.Close()
89-     })
90-     tuple.queryResult = queryResult
91-     status := C.lbug_query_result_get_next(&queryResult.cQueryResult, &tuple.cFlatTuple)
92-     if status != C.LbugSuccess {
93-             return tuple, fmt.Errorf("failed to get next tuple with status %d", status)
94-     }
95-     return tuple, nil
96-}
97-
98-// HasNextQueryResult returns true not all the query results is consumed when
99-// multiple query statements are executed.
100-func (queryResult *QueryResult) HasNextQueryResult() bool {
101-    return bool(C.lbug_query_result_has_next_query_result(&queryResult.cQueryResult))
102-}
103-
104-// NextQueryResult returns the next query result when multiple query statements are executed.
105-func (queryResult *QueryResult) NextQueryResult() (*QueryResult, error) {
106-    nextQueryResult := &QueryResult{}
107:    runtime.SetFinalizer(nextQueryResult, func(nextQueryResult *QueryResult) {
108-            nextQueryResult.Close()
109-    })
110-    status := C.lbug_query_result_get_next_query_result(&queryResult.cQueryResult, &nextQueryResult.cQueryResult)
111-    if status != C.LbugSuccess {
112-            return nextQueryResult, fmt.Errorf("failed to get next query result with status %d", status)
113-    }
114-    return nextQueryResult, nil
115-}
116-
117-// GetCompilingTime returns the compiling time of the query in milliseconds.

database.go
56-
57-// Database represents a Lbug database instance.
58-type Database struct {
59-     cDatabase C.lbug_database
60-     isClosed  bool
61-}
62-
63-// OpenDatabase opens a Lbug database at the given path with the given system configuration.
64-func OpenDatabase(path string, systemConfig SystemConfig) (*Database, error) {
65-     db := &Database{}
66:     runtime.SetFinalizer(db, func(db *Database) {
67-             db.Close()
68-     })
69-     cPath := C.CString(path)
70-     defer C.free(unsafe.Pointer(cPath))
71-     cSystemConfig := systemConfig.toC()
72-     status := C.lbug_database_init(cPath, cSystemConfig, &db.cDatabase)
73-     if status != C.LbugSuccess {
74-             return db, fmt.Errorf("failed to open database with status %d", status)
75-     }
76-     return db, nil

connection.go
14-type Connection struct {
15-     cConnection C.lbug_connection
16-     database    *Database
17-     isClosed    bool
18-}
19-
20-// OpenConnection opens a connection to the specified database.
21-func OpenConnection(database *Database) (*Connection, error) {
22-     conn := &Connection{}
23-     conn.database = database
24:     runtime.SetFinalizer(conn, func(conn *Connection) {
25-             conn.Close()
26-     })
27-     status := C.lbug_connection_init(&database.cDatabase, &conn.cConnection)
28-     if status != C.LbugSuccess {
29-             return conn, fmt.Errorf("failed to open connection with status %d", status)
30-     }
31-     return conn, nil
32-}
33-
34-// Close closes the Connection. Calling this method is optional.
--
66-func (conn *Connection) SetTimeout(timeout uint64) {
67-     C.lbug_connection_set_query_timeout(&conn.cConnection, C.uint64_t(timeout))
68-}
69-
70-// Query executes the specified query string and returns the result.
71-func (conn *Connection) Query(query string) (*QueryResult, error) {
72-     cQuery := C.CString(query)
73-     defer C.free(unsafe.Pointer(cQuery))
74-     queryResult := &QueryResult{}
75-     queryResult.connection = conn
76:     runtime.SetFinalizer(queryResult, func(queryResult *QueryResult) {
77-             queryResult.Close()
78-     })
79-     status := C.lbug_connection_query(&conn.cConnection, cQuery, &queryResult.cQueryResult)
80-     if status != C.LbugSuccess || !C.lbug_query_result_is_success(&queryResult.cQueryResult) {
81-             cErrMsg := C.lbug_query_result_get_error_message(&queryResult.cQueryResult)
82-             defer C.lbug_destroy_string(cErrMsg)
83-             return queryResult, fmt.Errorf(C.GoString(cErrMsg))
84-     }
85-     return queryResult, nil
86-}
--
89-// The arguments are a map of parameter names to values.
90-func (conn *Connection) Execute(preparedStatement *PreparedStatement, args map[string]any) (*QueryResult, error) {
91-     queryResult := &QueryResult{}
92-     queryResult.connection = conn
93-     for key, value := range args {
94-             err := conn.bindParameter(preparedStatement, key, value)
95-             if err != nil {
96-                     return queryResult, err
97-             }
98-     }
99:     runtime.SetFinalizer(queryResult, func(queryResult *QueryResult) {
100-            queryResult.Close()
101-    })
102-    status := C.lbug_connection_execute(&conn.cConnection, &preparedStatement.cPreparedStatement, &queryResult.cQueryResult)
103-    if status != C.LbugSuccess || !C.lbug_query_result_is_success(&queryResult.cQueryResult) {
104-            cErrMsg := C.lbug_query_result_get_error_message(&queryResult.cQueryResult)
105-            defer C.lbug_destroy_string(cErrMsg)
106-            return queryResult, fmt.Errorf(C.GoString(cErrMsg))
107-    }
108-    return queryResult, nil
109-}
--
127-    return nil
128-}
129-
130-// Prepare returns a prepared statement for the specified query string.
131-// The prepared statement can be used to execute the query with parameters.
132-func (conn *Connection) Prepare(query string) (*PreparedStatement, error) {
133-    cQuery := C.CString(query)
134-    defer C.free(unsafe.Pointer(cQuery))
135-    preparedStatement := &PreparedStatement{}
136-    preparedStatement.connection = conn
137:    runtime.SetFinalizer(preparedStatement, func(preparedStatement *PreparedStatement) {
138-            preparedStatement.Close()
139-    })
140-    status := C.lbug_connection_prepare(&conn.cConnection, cQuery, &preparedStatement.cPreparedStatement)
141-    if status != C.LbugSuccess || !C.lbug_prepared_statement_is_success(&preparedStatement.cPreparedStatement) {
142-            cErrMsg := C.lbug_prepared_statement_get_error_message(&preparedStatement.cPreparedStatement)
143-            defer C.lbug_destroy_string(cErrMsg)
144-            return preparedStatement, fmt.Errorf(C.GoString(cErrMsg))
145-    }
146-    return preparedStatement, nil
147-}

[adsharma]

@jkumz are you still looking into this?

[jkumz]

Yes, just been very busy but will get it done

…

On Wed, 4 Feb 2026 at 5:35 am, adsharma ***@***.***> wrote: *adsharma* left a comment (LadybugDB/go-ladybug#7) <#7 (comment)> @jkumz <https://github.com/jkumz> are you still looking into this? — Reply to this email directly, view it on GitHub <#7 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALRLDDKSX4I5I6OAYJ532WT4KEPEDAVCNFSM6AAAAACS2E5KBGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQNBUGEYDEMBSGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jkumz]

Ok, I tried a bunch of different approaches but it's turning more into a design decision than strictly debugging session.

The initial proposal above didn't work because KeepAlive(x) only keeps x reachable up to the point where KeepAlive is called. I tried to defer this to all methods that make CGO calls, but it still crashed in the finalizers because there is no ordering guarantees there.

When multiple objects in an ownership chain become unreachable simultaneously Go can run them in any order. The issue is definitely in the finalizers because commenting out the finalizer logic let tests pass (leaks memory instead).

sequenceDiagram
    participant GC as GC Cycle
    participant Conn as Connection Finalizer
    participant QR as QueryResult Finalizer
    
    Note over GC: All objects become unreachable
    GC->>Conn: Run finalizer (arbitrary order)
    Conn->>Conn: Close() → destroy connection
    Note over Conn: Connection C memory freed
    
    GC->>QR: Run finalizer
    QR->>QR: Close() → KeepAlive(connection)
    Note over QR: KeepAlive is useless, connection already destroyed
    QR->>QR: destroy query_result → SIGSEGV

Loading

The object ownership mode looks like this:

┌─────────────────────────────────────────────────┐
│ Database                                         │
│   ↑ (no ref)                                    │
│ Connection { database *Database }               │
│   ↑ (ref)                                       │
│ QueryResult { connection *Connection }          │
│   ↑ (ref)                                       │
│ FlatTuple { queryResult *QueryResult }          │
└─────────────────────────────────────────────────┘

So the next thing I tried was to replace the finalizers with runtime.AddCleanup, passing a pointer to the embedded C handle. This is what the docs for SetFinalizer recommend as an alternative because of the undeterministic nature of SetFinalizer. This also didn't work. The attempted fix here (code snippet below) panicked with runtime.AddCleanup: ptr is equal to arg, cleanup will never run.

runtime.AddCleanup(queryResult, func(cPtr *C.lbug_query_result) {
    C.lbug_query_result_destroy(cPtr)
}, &queryResult.cQueryResult)

This is because the C handle is embedded directly in the Go struct; AddCleanup detects that &queryResult.cQueryResult points inside the queryResult allocation and panics. When the Go object is collected the embedded field memory becomes invalid, so the cleanup would receive a dangling pointer.

I also tried to get around this using a wrapper struct which allocates the C handle in a separate heap object with this sort of pattern:

type cDatabaseHandle struct {
    cDatabase C.lbug_database
}

type Database struct {
    cHandle  *cDatabaseHandle  // Pointer to separate allocation
    isClosed bool
}

This failed with the following error:

fatal error: semawakeup on Darwin signal stack

goroutine 65 gp=0x14000444380 m=nil [runnable]:
runtime.AddCleanup[...].func1()
    /opt/homebrew/Cellar/go/1.25.7/libexec/src/runtime/mcleanup.go:101
runtime.runCleanups()
    /opt/homebrew/Cellar/go/1.25.7/libexec/src/runtime/mcleanup.go:665

After some further digging, runtime.AddCleanup runs clean up functions in a dedicated goroutine. Calling CGO functions from this goroutine seems to cause signal handling issues on MacOS (Darwin). Here's a related issue on this. On macOS, the Go runtime uses pthread mutexes for semaphore operations (semasleep/semawakeup). These aren't async-signal-safe. When a CGO function called from certain goroutine contexts triggers a signal or attempts lock operations, it crashes. I think this can be traced back to the C++ destructors.

// database.cpp
Database::~Database() {
    if (!dbConfig.readOnly && dbConfig.forceCheckpointOnClose) {
        try {
            ClientContext clientContext(this);
            transactionManager->checkpoint(clientContext);  // ← File I/O + pthread
        } catch (...) {}
    }
    dbLifeCycleManager->isDatabaseClosed = true;
}

// client_context.cpp  
ClientContext::~ClientContext() {
    if (!preventTransactionRollbackOnDestruction) {
        if (Transaction::Get(*this)) {
            getDatabase()->transactionManager->rollback(*this, Transaction::Get(*this));
        }
    }
}

When C.lbug_database_destroy() is called from AddCleanup:

1. delete Database* triggers Database::~Database()
2. Creates a ClientContext and calls checkpoint()
3. Checkpoint does file I/O (flushing WAL) and pthread synchronization
4. pthread operations from cleanup goroutine context crash on macOS

This seems to be a current macOS-specific runtime limitation. The Lbug C++ destructors require pthread operations, which are unsafe from the AddCleanup goroutine context on Darwin per this issue.

You can keep the automatic memory management if you use an MPSC channel approach to process clean up. Finalizers would only push to the channel and a single worker would process them respecting child-parent order. But I don't think this is a good trade-off because it adds a lot of excessive complexity and brings in its own issues that outweight the benefits in my opinion. There is a decent amount of (perhaps unneccessary) complexity in setting this up and keeping it orchestrated and maintaining child-parent order, but more importantly there is a deadlock risk with a buffered channel like this. With OLAP scale workloads its hard to size the buffer correctly as each user may have varying use cases and workloads.

Consider this hypothetical scenario:

1. User iterates N FlatTuples
2. They fall out of scope, GC runs
3. N finalizers fire and push to channel
4. Buffer fills up
5. Finalizer goroutine now blocks on any further pushes
6. Worker waiting for child counts to decrement (parent can't release until children are free to maintain child-parent order)
7. Child counts can't decrement because finalizer goroutine is blocked, so no new finalizers can fire
8. Deadlock

For a database it's probably not really worth the risk of deadlock in a language binding when the user can just do manual management with Close() calls. I think having to call Close() manually when there is CGo crossings is reasonable given GC doesn't collect C memory anyway.

Let me know your thoughts. I've attached test for reproduction below. Perhaps I have missed or overlooked something. Note that (at least in my experience) I only hit this issue when running the test as part of the test suite. This was the case in my own unit tests as well. Individually, no issue. As part of a larger test run it fails. So quite situational rather than a consistent bug, my guess is its related to GC pressure between tests, which would mean in real world use under high GC pressure you could have situations where this occurs.

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.

// 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) {
	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 := 0; g < numGoroutines; g++ {
		wg.Add(1)
		go func(goroutineID int) {
			defer wg.Done()

			for q := 0; q < queriesPerGoroutine; q++ {
				// Query without storing result in a variable that persists
				// This pattern allows the QueryResult to become "unreachable" quickly
				if err := runLargeQueryAndIterate(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 := i + batchSize
		if end > numNodes {
			end = 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()
		}
	}
}

// runLargeQueryAndIterate 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 runLargeQueryAndIterate(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)
	}
	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 := uint64(0); col < 7; col++ {
			_, err = row.GetValue(col)
			if err != nil {
				return fmt.Errorf("GetValue(%d) failed at row %d: %w", col, rowCount, err)
			}
		}

		rowCount++
	}

	return nil
}

[jkumz]

To clarify I've already got it fixed by making it so that user must call Close() rather than relying on finalizers, but this change would mean anybody who has been using it and not called Close would now need to or it would leak memory. I would say that's fair enough because I don't think there is that many people using the Go bindings right now, just needs a pr.

[adsharma]

People not calling result.Close() would result in leaking memory? Sounds like a more acceptable outcome than status quo. Thank you for looking into this.

[adsharma]

Got tied up with 0.15.0 release of the main repo. The change itself looks good, but I ran out of time trying to repro the race condition. Will try again later today.