Skip to content

Commit b70dcb0

Browse files
committed
Adapt to C API and fix potential handle leaks for callbacks
Also make context to manage context options lifetime and adapt related docs
1 parent 9a83c4c commit b70dcb0

File tree

5 files changed

+58
-94
lines changed

5 files changed

+58
-94
lines changed

kernel/chain.go

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -54,23 +54,6 @@ func (c *Chain) GetByHeight(height int) *BlockTreeEntry {
5454
return entry
5555
}
5656

57-
// GetNextBlockTreeEntry returns the next block tree entry in the active chain, or nil if block tree entry is the chain tip or not in the chain
58-
func (c *Chain) GetNextBlockTreeEntry(blockTreeEntry *BlockTreeEntry) (*BlockTreeEntry, error) {
59-
checkReady(c)
60-
if err := validateReady(blockTreeEntry); err != nil {
61-
return nil, err
62-
}
63-
64-
ptr := C.btck_chain_get_next_block_tree_entry(c.ptr, blockTreeEntry.ptr)
65-
if ptr == nil {
66-
return nil, nil
67-
}
68-
69-
nextEntry := &BlockTreeEntry{ptr: ptr}
70-
runtime.SetFinalizer(nextEntry, (*BlockTreeEntry).destroy)
71-
return nextEntry, nil
72-
}
73-
7457
// Contains returns true if the chain contains the block tree entry
7558
func (c *Chain) Contains(blockTreeEntry *BlockTreeEntry) bool {
7659
checkReady(c)

kernel/chain_test.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,20 +76,6 @@ func TestChain(t *testing.T) {
7676
t.Errorf("Expected block height 1, got %d", block1.Height())
7777
}
7878

79-
// Test GetNextBlockTreeEntry
80-
nextEntry, err := chain.GetNextBlockTreeEntry(genesis)
81-
if err != nil {
82-
t.Fatalf("Chain.GetNextBlockTreeEntry() error = %v", err)
83-
}
84-
if nextEntry == nil {
85-
t.Fatal("Next block tree entry is nil")
86-
}
87-
defer nextEntry.Destroy()
88-
89-
if nextEntry.Height() != 1 {
90-
t.Errorf("Expected next block height 1, got %d", nextEntry.Height())
91-
}
92-
9379
// Test Contains
9480
containsGenesis := chain.Contains(genesis)
9581
if !containsGenesis {

kernel/context.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,11 @@ var _ cManagedResource = &Context{}
1616
//
1717
// A constructed context can be safely used from multiple threads.
1818
type Context struct {
19-
ptr *C.btck_Context
19+
ptr *C.btck_Context
20+
options *ContextOptions
2021
}
2122

2223
// NewContext creates a new kernel context with the specified options.
23-
// Kernel copies all necessary data from the options during context creation,
24-
// so the caller can safely free the options object after this call returns.
2524
func NewContext(options *ContextOptions) (*Context, error) {
2625
if err := validateReady(options); err != nil {
2726
return nil, err
@@ -32,21 +31,20 @@ func NewContext(options *ContextOptions) (*Context, error) {
3231
return nil, ErrKernelContextCreate
3332
}
3433

35-
ctx := &Context{ptr: ptr}
34+
ctx := &Context{
35+
ptr: ptr,
36+
options: options,
37+
}
3638
runtime.SetFinalizer(ctx, (*Context).destroy)
3739
return ctx, nil
3840
}
3941

4042
// NewDefaultContext creates a new kernel context with default mainnet parameters.
41-
// The defer statements are safe here because the Kernel copies all necessary
42-
// data during context creation, so the caller can safely free the options and
43-
// parameters objects immediately after the context is created.
4443
func NewDefaultContext() (*Context, error) {
4544
opts, err := NewContextOptions()
4645
if err != nil {
4746
return nil, err
4847
}
49-
defer opts.Destroy()
5048

5149
params, err := NewChainParameters(ChainTypeMainnet)
5250
if err != nil {
@@ -69,6 +67,10 @@ func (ctx *Context) destroy() {
6967
C.btck_context_destroy(ctx.ptr)
7068
ctx.ptr = nil
7169
}
70+
if ctx.options != nil {
71+
ctx.options.Destroy()
72+
ctx.options = nil
73+
}
7274
}
7375

7476
func (ctx *Context) Destroy() {

kernel/context_options.go

Lines changed: 38 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -15,42 +15,18 @@ extern void go_notify_warning_unset_bridge(void* user_data, btck_Warning warning
1515
extern void go_notify_flush_error_bridge(void* user_data, const char* message, size_t message_len);
1616
extern void go_notify_fatal_error_bridge(void* user_data, const char* message, size_t message_len);
1717
extern void go_validation_interface_block_checked_bridge(void* user_data, const btck_BlockPointer* block, const btck_BlockValidationState* state);
18-
19-
// Wrapper function: C helper to set notifications with Go callbacks
20-
// Converts Handle ID to void* and passes to C library
21-
static inline void set_notifications_wrapper(btck_ContextOptions* opts, uintptr_t handle) {
22-
btck_NotificationInterfaceCallbacks callbacks = {
23-
.user_data = (void*)handle,
24-
.block_tip = (btck_NotifyBlockTip)go_notify_block_tip_bridge,
25-
.header_tip = (btck_NotifyHeaderTip)go_notify_header_tip_bridge,
26-
.progress = (btck_NotifyProgress)go_notify_progress_bridge,
27-
.warning_set = (btck_NotifyWarningSet)go_notify_warning_set_bridge,
28-
.warning_unset = (btck_NotifyWarningUnset)go_notify_warning_unset_bridge,
29-
.flush_error = (btck_NotifyFlushError)go_notify_flush_error_bridge,
30-
.fatal_error = (btck_NotifyFatalError)go_notify_fatal_error_bridge,
31-
};
32-
btck_context_options_set_notifications(opts, callbacks);
33-
}
34-
35-
// Wrapper function: C helper to set validation interface with Go callbacks
36-
// Converts Handle ID to void* and passes to C library
37-
static inline void set_validation_interface_wrapper(btck_ContextOptions* opts, uintptr_t handle) {
38-
btck_ValidationInterfaceCallbacks callbacks = {
39-
.user_data = (void*)handle,
40-
.block_checked = (btck_ValidationInterfaceBlockChecked)go_validation_interface_block_checked_bridge,
41-
};
42-
btck_context_options_set_validation_interface(opts, callbacks);
43-
}
4418
*/
4519
import "C"
4620
import (
4721
"runtime"
4822
"runtime/cgo"
23+
"unsafe"
4924
)
5025

5126
var _ cManagedResource = &ContextOptions{}
5227

53-
// ContextOptions wraps the C btck_ContextOptions
28+
// ContextOptions wraps the C btck_ContextOptions.
29+
// Once the options is set on a context, the context is responsible for its lifetime and should not be destroyed manually.
5430
type ContextOptions struct {
5531
ptr *C.btck_ContextOptions
5632
notificationHandle cgo.Handle // Prevents notification callbacks GC until Destroy() called
@@ -87,15 +63,29 @@ func (opts *ContextOptions) SetNotifications(callbacks *NotificationCallbacks) e
8763
return ErrNilNotificationCallbacks
8864
}
8965

66+
// Clean up existing handle if present
67+
if opts.notificationHandle != 0 {
68+
opts.notificationHandle.Delete()
69+
opts.notificationHandle = 0
70+
}
71+
9072
// Create a handle for the callbacks - this prevents garbage collection
9173
// and provides a stable ID that can be passed through C code safely
92-
handle := cgo.NewHandle(callbacks)
93-
94-
// Call the C wrapper function to set all notification callbacks
95-
C.set_notifications_wrapper(opts.ptr, C.uintptr_t(handle))
96-
97-
// Store the handle to prevent GC and allow cleanup
98-
opts.notificationHandle = handle
74+
opts.notificationHandle = cgo.NewHandle(callbacks)
75+
76+
// Create notification callbacks struct and call C library directly
77+
notificationCallbacks := C.btck_NotificationInterfaceCallbacks{
78+
user_data: unsafe.Pointer(opts.notificationHandle),
79+
user_data_destroy: nil, // Go handles memory management via cgo.Handle
80+
block_tip: C.btck_NotifyBlockTip(C.go_notify_block_tip_bridge),
81+
header_tip: C.btck_NotifyHeaderTip(C.go_notify_header_tip_bridge),
82+
progress: C.btck_NotifyProgress(C.go_notify_progress_bridge),
83+
warning_set: C.btck_NotifyWarningSet(C.go_notify_warning_set_bridge),
84+
warning_unset: C.btck_NotifyWarningUnset(C.go_notify_warning_unset_bridge),
85+
flush_error: C.btck_NotifyFlushError(C.go_notify_flush_error_bridge),
86+
fatal_error: C.btck_NotifyFatalError(C.go_notify_fatal_error_bridge),
87+
}
88+
C.btck_context_options_set_notifications(opts.ptr, notificationCallbacks)
9989
return nil
10090
}
10191

@@ -107,15 +97,23 @@ func (opts *ContextOptions) SetValidationInterface(callbacks *ValidationInterfac
10797
return ErrNilValidationInterfaceCallbacks
10898
}
10999

100+
// Clean up existing handle if present
101+
if opts.validationHandle != 0 {
102+
opts.validationHandle.Delete()
103+
opts.validationHandle = 0
104+
}
105+
110106
// Create a handle for the callbacks - this prevents garbage collection
111107
// and provides a stable ID that can be passed through C code safely
112-
handle := cgo.NewHandle(callbacks)
108+
opts.validationHandle = cgo.NewHandle(callbacks)
113109

114-
// Call the C wrapper function to set all validation interface callbacks
115-
C.set_validation_interface_wrapper(opts.ptr, C.uintptr_t(handle))
116-
117-
// Store the handle to prevent GC and allow cleanup
118-
opts.validationHandle = handle
110+
// Create validation callbacks struct and call C library directly
111+
validationCallbacks := C.btck_ValidationInterfaceCallbacks{
112+
user_data: unsafe.Pointer(opts.validationHandle),
113+
user_data_destroy: nil, // Go handles memory management via cgo.Handle
114+
block_checked: C.btck_ValidationInterfaceBlockChecked(C.go_validation_interface_block_checked_bridge),
115+
}
116+
C.btck_context_options_set_validation_interface(opts.ptr, validationCallbacks)
119117
return nil
120118
}
121119

kernel/logging_connection.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,6 @@ package kernel
88
// Bridge function: exported Go function that C library can call
99
// user_data contains the cgo.Handle ID as void* for callback identification
1010
extern void go_log_callback_bridge(void* user_data, char* message, size_t message_len);
11-
12-
// Wrapper function: C helper to create logging connection with Go callback
13-
// Converts Handle ID to void* and passes to C library
14-
static inline btck_LoggingConnection* create_logging_connection_wrapper(uintptr_t context, btck_LoggingOptions options) {
15-
return btck_logging_connection_create((btck_LogCallback)go_log_callback_bridge, (void*)context, options);
16-
}
1711
*/
1812
import "C"
1913
import (
@@ -62,9 +56,14 @@ func NewLoggingConnection(callback LogCallback, options LoggingOptions) (*Loggin
6256
return nil, ErrLoggingConnectionUninitialized //FIXME
6357
}
6458

59+
connection := &LoggingConnection{
60+
ptr: nil,
61+
handle: 0,
62+
}
63+
6564
// Create a handle for the callback - this prevents garbage collection
6665
// and provides a stable ID that can be passed through C code safely
67-
handle := cgo.NewHandle(callback)
66+
connection.handle = cgo.NewHandle(callback)
6867

6968
cOptions := C.btck_LoggingOptions{
7069
log_timestamps: boolToInt(options.LogTimestamps),
@@ -74,17 +73,13 @@ func NewLoggingConnection(callback LogCallback, options LoggingOptions) (*Loggin
7473
always_print_category_levels: boolToInt(options.AlwaysPrintCategoryLevel),
7574
}
7675

77-
ptr := C.create_logging_connection_wrapper(C.uintptr_t(handle), cOptions)
78-
if ptr == nil {
79-
handle.Delete()
76+
connection.ptr = C.btck_logging_connection_create((C.btck_LogCallback)(C.go_log_callback_bridge),
77+
unsafe.Pointer(connection.handle), nil, cOptions)
78+
if connection.ptr == nil {
79+
connection.handle.Delete()
8080
return nil, ErrKernelLoggingConnectionCreate
8181
}
8282

83-
connection := &LoggingConnection{
84-
ptr: ptr,
85-
handle: handle,
86-
}
87-
8883
runtime.SetFinalizer(connection, (*LoggingConnection).destroy)
8984
return connection, nil
9085
}

0 commit comments

Comments
 (0)