internal/glfw: Bug fix: NewCallbackCDecl objects were leaked

Callbacks created by NewCallbackCDecl were never released and then
they are leaked, especially when the window size was changed by
SetWindowSize on Windows.

This change defines new callback ID types with uintptr, and reuse
the callbacks.

Closes #1672

internal/glfw/callback_notwindows.go

@@ -0,0 +1,82 @@
+// Copyright 2021 The Ebiten Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build !windows
+// +build !windows
+
+package glfw
+
+import (
+	"github.com/go-gl/glfw/v3.3/glfw"
+)
+
+var (
+	/*charModsCallbacks        = map[CharModsCallback]func(window *Window, char rune, mods ModifierKey){}
+	framebufferSizeCallbacks = map[FramebufferSizeCallback]func(window *Window, width int, height int){}
+	scrollCallbacks          = map[ScrollCallback]func(window *Window, xoff float64, yoff float64){}
+	sizeCallbacks            = map[SizeCallback]func(window *Window, width int, height int){}*/
+
+	charModsCallbacks        = map[CharModsCallback]glfw.CharModsCallback{}
+	framebufferSizeCallbacks = map[FramebufferSizeCallback]glfw.FramebufferSizeCallback{}
+	scrollCallbacks          = map[ScrollCallback]glfw.ScrollCallback{}
+	sizeCallbacks            = map[SizeCallback]glfw.SizeCallback{}
+)
+
+func ToCharModsCallback(cb func(window *Window, char rune, mods ModifierKey)) CharModsCallback {
+	if cb == nil {
+		return 0
+	}
+	id := CharModsCallback(len(charModsCallbacks) + 1)
+	var gcb glfw.CharModsCallback = func(window *glfw.Window, char rune, mods glfw.ModifierKey) {
+		cb(theWindows.get(window), char, ModifierKey(mods))
+	}
+	charModsCallbacks[id] = gcb
+	return id
+}
+
+func ToFramebufferSizeCallback(cb func(window *Window, width int, height int)) FramebufferSizeCallback {
+	if cb == nil {
+		return 0
+	}
+	id := FramebufferSizeCallback(len(framebufferSizeCallbacks) + 1)
+	var gcb glfw.FramebufferSizeCallback = func(window *glfw.Window, width int, height int) {
+		cb(theWindows.get(window), width, height)
+	}
+	framebufferSizeCallbacks[id] = gcb
+	return id
+}
+
+func ToScrollCallback(cb func(window *Window, xoff float64, yoff float64)) ScrollCallback {
+	if cb == nil {
+		return 0
+	}
+	id := ScrollCallback(len(scrollCallbacks) + 1)
+	var gcb glfw.ScrollCallback = func(window *glfw.Window, xoff float64, yoff float64) {
+		cb(theWindows.get(window), xoff, yoff)
+	}
+	scrollCallbacks[id] = gcb
+	return id
+}
+
+func ToSizeCallback(cb func(window *Window, width int, height int)) SizeCallback {
+	if cb == nil {
+		return 0
+	}
+	id := SizeCallback(len(sizeCallbacks) + 1)
+	var gcb glfw.SizeCallback = func(window *glfw.Window, width, height int) {
+		cb(theWindows.get(window), width, height)
+	}
+	sizeCallbacks[id] = gcb
+	return id
+}

internal/glfw/callback_windows.go

@@ -0,0 +1,61 @@
+// Copyright 2021 The Ebiten Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package glfw
+
+import (
+	"golang.org/x/sys/windows"
+)
+
+func ToCharModsCallback(cb func(window *Window, char rune, mods ModifierKey)) CharModsCallback {
+	if cb == nil {
+		return 0
+	}
+	return CharModsCallback(windows.NewCallbackCDecl(func(window uintptr, char rune, mods ModifierKey) uintptr {
+		cb(theGLFWWindows.get(window), char, mods)
+		return 0
+	}))
+}
+
+func ToFramebufferSizeCallback(cb func(window *Window, width int, height int)) FramebufferSizeCallback {
+	if cb == nil {
+		return 0
+	}
+	return FramebufferSizeCallback(windows.NewCallbackCDecl(func(window uintptr, width int, height int) uintptr {
+		cb(theGLFWWindows.get(window), width, height)
+		return 0
+	}))
+}
+
+func ToScrollCallback(cb func(window *Window, xoff float64, yoff float64)) ScrollCallback {
+	if cb == nil {
+		return 0
+	}
+	return ScrollCallback(windows.NewCallbackCDecl(func(window uintptr, xoff *float64, yoff *float64) uintptr {
+		// xoff and yoff were originally float64, but there is no good way to pass them on 32bit
+		// machines via NewCallback. We've fixed GLFW side to use pointer values.
+		cb(theGLFWWindows.get(window), *xoff, *yoff)
+		return 0
+	}))
+}
+
+func ToSizeCallback(cb func(window *Window, width int, height int)) SizeCallback {
+	if cb == nil {
+		return 0
+	}
+	return SizeCallback(windows.NewCallbackCDecl(func(window uintptr, width int, height int) uintptr {
+		cb(theGLFWWindows.get(window), width, height)
+		return 0
+	}))
+}

internal/glfw/glfw_notwindows.go

@@ -162,14 +162,8 @@ func (w *Window) SetAttrib(attrib Hint, value int) {
 }
 
 func (w *Window) SetCharModsCallback(cbfun CharModsCallback) (previous CharModsCallback) {
-	var gcb glfw.CharModsCallback
+	w.w.SetCharModsCallback(charModsCallbacks[cbfun])
-	if cbfun != nil {
+	return 0 // TODO
-		gcb = func(window *glfw.Window, char rune, mods glfw.ModifierKey) {
-			cbfun(theWindows.get(window), char, ModifierKey(mods))
-		}
-	}
-	w.w.SetCharModsCallback(gcb)
-	return nil // TODO
 }
 
 func (w *Window) SetCursor(cursor *Cursor) {
@@ -181,35 +175,17 @@ func (w *Window) SetCursor(cursor *Cursor) {
 }
 
 func (w *Window) SetFramebufferSizeCallback(cbfun FramebufferSizeCallback) (previous FramebufferSizeCallback) {
-	var gcb glfw.FramebufferSizeCallback
+	w.w.SetFramebufferSizeCallback(framebufferSizeCallbacks[cbfun])
-	if cbfun != nil {
+	return 0 // TODO
-		gcb = func(window *glfw.Window, width int, height int) {
-			cbfun(theWindows.get(window), width, height)
-		}
-	}
-	w.w.SetFramebufferSizeCallback(gcb)
-	return nil // TODO
 }
 
 func (w *Window) SetScrollCallback(cbfun ScrollCallback) (previous ScrollCallback) {
-	var gcb glfw.ScrollCallback
+	w.w.SetScrollCallback(scrollCallbacks[cbfun])
-	if cbfun != nil {
+	return 0 // TODO
-		gcb = func(window *glfw.Window, xoff float64, yoff float64) {
-			cbfun(theWindows.get(window), xoff, yoff)
-		}
-	}
-	w.w.SetScrollCallback(gcb)
-	return nil // TODO
 }
 
 func (w *Window) SetSizeCallback(cbfun SizeCallback) (previous SizeCallback) {
-	var gcb glfw.SizeCallback
+	w.w.SetSizeCallback(sizeCallbacks[cbfun])
-	if cbfun != nil {
-		gcb = func(window *glfw.Window, width, height int) {
-			cbfun(theWindows.get(window), width, height)
-		}
-	}
-	w.w.SetSizeCallback(gcb)
 	prev := w.prevSizeCallback
 	w.prevSizeCallback = cbfun
 	return prev

internal/glfw/glfw_windows.go

@@ -196,16 +196,9 @@ func (w *Window) Restore() {
 }
 
 func (w *Window) SetCharModsCallback(cbfun CharModsCallback) (previous CharModsCallback) {
-	var gcb uintptr
+	glfwDLL.call("glfwSetCharModsCallback", w.w, uintptr(cbfun))
-	if cbfun != nil {
-		gcb = windows.NewCallbackCDecl(func(window uintptr, char rune, mods ModifierKey) uintptr {
-			cbfun(theGLFWWindows.get(window), char, mods)
-			return 0
-		})
-	}
-	glfwDLL.call("glfwSetCharModsCallback", w.w, gcb)
 	panicError()
-	return nil // TODO
+	return ToCharModsCallback(nil) // TODO
 }
 
 func (w *Window) SetCursor(cursor *Cursor) {
@@ -217,42 +210,19 @@ func (w *Window) SetCursor(cursor *Cursor) {
 }
 
 func (w *Window) SetFramebufferSizeCallback(cbfun FramebufferSizeCallback) (previous FramebufferSizeCallback) {
-	var gcb uintptr
+	glfwDLL.call("glfwSetFramebufferSizeCallback", w.w, uintptr(cbfun))
-	if cbfun != nil {
-		gcb = windows.NewCallbackCDecl(func(window uintptr, width int, height int) uintptr {
-			cbfun(theGLFWWindows.get(window), width, height)
-			return 0
-		})
-	}
-	glfwDLL.call("glfwSetFramebufferSizeCallback", w.w, gcb)
 	panicError()
-	return nil // TODO
+	return ToFramebufferSizeCallback(nil) // TODO
 }
 
 func (w *Window) SetScrollCallback(cbfun ScrollCallback) (previous ScrollCallback) {
-	var gcb uintptr
+	glfwDLL.call("glfwSetScrollCallback", w.w, uintptr(cbfun))
-	if cbfun != nil {
-		gcb = windows.NewCallbackCDecl(func(window uintptr, xoff *float64, yoff *float64) uintptr {
-			// xoff and yoff were originally float64, but there is no good way to pass them on 32bit
-			// machines via NewCallback. We've fixed GLFW side to use pointer values.
-			cbfun(theGLFWWindows.get(window), *xoff, *yoff)
-			return 0
-		})
-	}
-	glfwDLL.call("glfwSetScrollCallback", w.w, gcb)
 	panicError()
-	return nil // TODO
+	return ToScrollCallback(nil) // TODO
 }
 
 func (w *Window) SetSizeCallback(cbfun SizeCallback) (previous SizeCallback) {
-	var gcb uintptr
+	glfwDLL.call("glfwSetWindowSizeCallback", w.w, uintptr(cbfun))
-	if cbfun != nil {
-		gcb = windows.NewCallbackCDecl(func(window uintptr, width int, height int) uintptr {
-			cbfun(theGLFWWindows.get(window), width, height)
-			return 0
-		})
-	}
-	glfwDLL.call("glfwSetWindowSizeCallback", w.w, gcb)
 	panicError()
 	prev := w.prevSizeCallback
 	w.prevSizeCallback = cbfun

internal/glfw/type.go

@@ -18,10 +18,10 @@
 package glfw
 
 type (
-	CharModsCallback        func(w *Window, char rune, mods ModifierKey)
+	CharModsCallback        uintptr
-	FramebufferSizeCallback func(w *Window, width int, height int)
+	FramebufferSizeCallback uintptr
-	ScrollCallback          func(w *Window, xoff float64, yoff float64)
+	ScrollCallback          uintptr
-	SizeCallback            func(w *Window, width int, height int)
+	SizeCallback            uintptr
 )
 
 type VidMode struct {

internal/uidriver/glfw/input.go

@@ -273,7 +273,7 @@ func (i *Input) update(window *glfw.Window, context driver.UIContext) {
 	defer i.ui.m.Unlock()
 
 	i.onceCallback.Do(func() {
-		window.SetCharModsCallback(func(w *glfw.Window, char rune, mods glfw.ModifierKey) {
+		window.SetCharModsCallback(glfw.ToCharModsCallback(func(w *glfw.Window, char rune, mods glfw.ModifierKey) {
 			// As this function is called from GLFW callbacks, the current thread is main.
 			if !unicode.IsPrint(char) {
 				return
@@ -282,14 +282,14 @@ func (i *Input) update(window *glfw.Window, context driver.UIContext) {
 			i.ui.m.Lock()
 			defer i.ui.m.Unlock()
 			i.runeBuffer = append(i.runeBuffer, char)
-		})
+		}))
-		window.SetScrollCallback(func(w *glfw.Window, xoff float64, yoff float64) {
+		window.SetScrollCallback(glfw.ToScrollCallback(func(w *glfw.Window, xoff float64, yoff float64) {
 			// As this function is called from GLFW callbacks, the current thread is main.
 			i.ui.m.Lock()
 			defer i.ui.m.Unlock()
 			i.scrollX = xoff
 			i.scrollY = yoff
-		})
+		}))
 	})
 	if i.keyPressed == nil {
 		i.keyPressed = map[glfw.Key]bool{}

internal/uidriver/glfw/ui.go

@@ -107,6 +107,10 @@ type UserInterface struct {
 	input   Input
 	iwindow window
 
+	sizeCallback              glfw.SizeCallback
+	framebufferSizeCallback   glfw.FramebufferSizeCallback
+	framebufferSizeCallbackCh chan struct{}
+
 	t thread.Thread
 	m sync.RWMutex
 }
@@ -726,47 +730,50 @@ func (u *UserInterface) createWindow() error {
 
 // registerWindowSetSizeCallback must be called from the main thread.
 func (u *UserInterface) registerWindowSetSizeCallback() {
-	u.window.SetSizeCallback(func(_ *glfw.Window, width, height int) {
+	if u.sizeCallback == 0 {
-		if !u.setSizeCallbackEnabled {
+		u.sizeCallback = glfw.ToSizeCallback(func(_ *glfw.Window, width, height int) {
-			return
+			if !u.setSizeCallbackEnabled {
-		}
+				return
-
-		if u.window.GetAttrib(glfw.Resizable) == glfw.False {
-			return
-		}
-		if u.isFullscreen() {
-			return
-		}
-
-		if err := u.runOnAnotherThreadFromMainThread(func() error {
-			var outsideWidth, outsideHeight float64
-			var outsideSizeChanged bool
-
-			_ = u.t.Call(func() error {
-				if width != 0 || height != 0 {
-					u.setWindowSize(width, height, u.isFullscreen())
-				}
-
-				outsideWidth, outsideHeight, outsideSizeChanged = u.updateSize()
-				return nil
-			})
-			if outsideSizeChanged {
-				u.context.Layout(outsideWidth, outsideHeight)
 			}
-			if err := u.context.ForceUpdate(); err != nil {
+
-				return err
+			if u.window.GetAttrib(glfw.Resizable) == glfw.False {
+				return
 			}
-			if u.Graphics().IsGL() {
+			if u.isFullscreen() {
+				return
+			}
+
+			if err := u.runOnAnotherThreadFromMainThread(func() error {
+				var outsideWidth, outsideHeight float64
+				var outsideSizeChanged bool
+
 				_ = u.t.Call(func() error {
-					u.swapBuffers()
+					if width != 0 || height != 0 {
+						u.setWindowSize(width, height, u.isFullscreen())
+					}
+
+					outsideWidth, outsideHeight, outsideSizeChanged = u.updateSize()
 					return nil
 				})
+				if outsideSizeChanged {
+					u.context.Layout(outsideWidth, outsideHeight)
+				}
+				if err := u.context.ForceUpdate(); err != nil {
+					return err
+				}
+				if u.Graphics().IsGL() {
+					_ = u.t.Call(func() error {
+						u.swapBuffers()
+						return nil
+					})
+				}
+				return nil
+			}); err != nil {
+				u.err = err
 			}
-			return nil
+		})
-		}); err != nil {
+	}
-			u.err = err
+	u.window.SetSizeCallback(u.sizeCallback)
-		}
-	})
 }
 
 func (u *UserInterface) init() error {
@@ -1210,15 +1217,18 @@ func (u *UserInterface) setWindowSize(width, height int, fullscreen bool) {
 		newW := width
 		newH := height
 		if oldW != newW || oldH != newH {
-			ch := make(chan struct{}, 1)
+			u.framebufferSizeCallbackCh = make(chan struct{}, 1)
-			u.window.SetFramebufferSizeCallback(func(_ *glfw.Window, _, _ int) {
+			if u.framebufferSizeCallback == 0 {
-				// This callback can be invoked multiple times by one PollEvents in theory (#1618).
+				u.framebufferSizeCallback = glfw.ToFramebufferSizeCallback(func(_ *glfw.Window, _, _ int) {
-				// Allow the case when the channel is full.
+					// This callback can be invoked multiple times by one PollEvents in theory (#1618).
-				select {
+					// Allow the case when the channel is full.
-				case ch <- struct{}{}:
+					select {
-				default:
+					case u.framebufferSizeCallbackCh <- struct{}{}:
-				}
+					default:
-			})
+					}
+				})
+			}
+			u.window.SetFramebufferSizeCallback(u.framebufferSizeCallback)
 			u.window.SetSize(newW, newH)
 			// Just after SetSize, GetSize is not reliable especially on Linux/UNIX.
 			// Let's wait for FramebufferSize callback in any cases.
@@ -1231,7 +1241,7 @@ func (u *UserInterface) setWindowSize(width, height int, fullscreen bool) {
 			for {
 				glfw.PollEvents()
 				select {
-				case <-ch:
+				case <-u.framebufferSizeCallbackCh:
 					break event
 				case <-t.C:
 					break event
@@ -1239,8 +1249,9 @@ func (u *UserInterface) setWindowSize(width, height int, fullscreen bool) {
 					time.Sleep(time.Millisecond)
 				}
 			}
-			u.window.SetFramebufferSizeCallback(nil)
+			u.window.SetFramebufferSizeCallback(glfw.ToFramebufferSizeCallback(nil))
-			close(ch)
+			close(u.framebufferSizeCallbackCh)
+			u.framebufferSizeCallbackCh = nil
 		}
 
 		// Window title might be lost on macOS after coming back from fullscreen.