Skip to content

Possible memory leak of c_name_ptr when SetMouseHandler is called more than once on a window #1379

Description

@OvOhao

Possible memory leak of c_name_ptr when SetMouseHandler is called more than once on a window

I found a possible memory leak in (*Window).SetMouseHandler. The method allocates a C.CString
for the window name and records it in the package-level onMouseHandlers map keyed by that name.
Calling it a second time for the same window overwrites the map entry without freeing the previous
allocation, and Close — the only place the pointer is ever released — can see only the most recent
entry. Re-registering a mouse handler is ordinary usage (for example switching a tool between a
"draw rectangle" and a "pick colour" mode), so each re-registration permanently leaks one
allocation.

File: highgui.go

Function: gocv.io/x/gocv.(*Window).SetMouseHandler (highgui.go:409-418)

func (w *Window) SetMouseHandler(onMOuse MouseHandlerFunc, userdata interface{}) {
	c_winname := C.CString(w.name)

	onMouseHandlers[w.name] = mouseHandlerInfo{
		c_name_ptr: c_winname,
		fn:         onMOuse,
		userdata:   userdata,
	}

	C.Window_SetMouseCallback(c_winname, C.mouse_callback(C.go_onmouse_dispatcher))
}

The registry (highgui.go:20-28):

type mouseHandlerInfo struct {
	c_name_ptr *C.char
	fn         MouseHandlerFunc
	userdata   interface{}
}

var (
	onMouseHandlers = map[string]mouseHandlerInfo{}
)

and the only release site, (*Window).Close (highgui.go:62-75):

func (w *Window) Close() error {
	cName := C.CString(w.name)
	defer C.free(unsafe.Pointer(cName))

	mcbInfo, exists := onMouseHandlers[w.name]
	if exists {
		mcbInfo.fn = nil
		mcbInfo.userdata = nil
		C.free(unsafe.Pointer(mcbInfo.c_name_ptr))
		delete(onMouseHandlers, w.name)
	}

	C.Window_Close(cName)
  1. Application code calls win.SetMouseHandler(fn, ud) on a *Window.
  2. Line 410 calls C.CString(w.name), allocating len(w.name)+1 bytes with malloc.
  3. The pointer is stored twice: into onMouseHandlers[w.name].c_name_ptr (the Go-side ownership
    record) and into OpenCV itself as the callback userdata, via C.Window_SetMouseCallback,
    whose C++ body is cv::setMouseCallback(winname, on_mouse, (void*)winname)
    (highgui.cpp:5-11) — it passes the same pointer as both the name and the user data.
  4. On a second call for the same window, line 412 assigns a fresh mouseHandlerInfo to the
    same map key. Go map assignment replaces the value outright: the previous c_name_ptr is not
    read, not freed, and is now unreachable from Go. Close at line 70 can only ever free the entry
    currently in the map.
  5. N registrations on one window therefore leak N-1 malloc blocks. grep -rn "c_name_ptr"
    over the repository returns exactly three sites — the field declaration (line 21), the free in
    Close (line 70), and this assignment (line 413) — so no other code can release the orphans.
    OpenCV does not free callback user data either.

Go trigger (if applicable):

win := gocv.NewWindow("demo")
defer win.Close()

for i := 0; i < 1_000_000; i++ {
	// Each iteration allocates a fresh C string for "demo" and overwrites
	// onMouseHandlers["demo"], orphaning the previous one.
	win.SetMouseHandler(func(event, x, y, flags int, ud interface{}) {}, nil)
}
// Close() frees only the last allocation; 999,999 blocks are leaked.

No mouse event, goroutine or GC step is required — the leak happens on the registration path alone,
and Go's collector never reclaims malloc memory. It requires an OpenCV HighGUI build.

Separately, onMouseHandlers is an unsynchronised package-level map that is read from the C
callback thread in go_onmouse_dispatcher (line 402) and written from application goroutines
(line 412); that data race is a distinct issue from the leak reported here.

Suggested fix: free any existing entry before overwriting it, e.g.

func (w *Window) SetMouseHandler(onMOuse MouseHandlerFunc, userdata interface{}) {
	if old, ok := onMouseHandlers[w.name]; ok {
		C.free(unsafe.Pointer(old.c_name_ptr))
	}
	c_winname := C.CString(w.name)
	...
}

A more robust alternative is to replace the raw *C.char user data with a runtime/cgo.Handle,
whose lifetime is explicit and which also removes the need to pass a Go-visible pointer to OpenCV.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions