ncruces/zenity

crashes on syscall.Syscall6

gnoxia opened this issue · 3 comments

Hi,I've encounter crash on updating zenity.Progress, it happens frequently when fast updating progress dialog value.

env: windows 2012 server
go: 1.18
ui: zenity.progress

crash stacks:

Exception 0xc0000005 0x0 0xc00063b8c8 0x7ffe000f4d56
PC=0x7ffe000f4d56

runtime.cgocall(0x41b9e0, 0xc000066ec0)
	/opt/homebrew/Cellar/go/1.18.3/libexec/src/runtime/cgocall.go:157 +0x4a fp=0xc0004d9780 sp=0xc0004d9748 pc=0x3b458a
syscall.SyscallN(0xd45f58?, {0xc00063b818?, 0x41685b?, 0x7ffe000e1970?})
	/opt/homebrew/Cellar/go/1.18.3/libexec/src/runtime/syscall_windows.go:538 +0x109 fp=0xc0004d97f8 sp=0xc0004d9780 pc=0x416c49
syscall.Syscall6(0xc00013ca20?, 0x23d7b634e19?, 0x23d7b634e19?, 0x0?, 0xc0004d98f0?, 0x3bdda5?, 0x23d744a0108?, 0xc0004d9908?)
	/opt/homebrew/Cellar/go/1.18.3/libexec/src/runtime/syscall_windows.go:482 +0x50 fp=0xc0004d9858 sp=0xc0004d97f8 pc=0x4168d0
github.com/ncruces/zenity/internal/win.MessageLoop(0xc0002f0340?)
	/Users/test/go/pkg/mod/github.com/ncruces/zenity@v0.8.8/internal/win/user32.go:307 +0x152 fp=0xc0004d9940 sp=0xc0004d9858 pc=0x703c12
github.com/ncruces/zenity.(*progressDialog).setup(_, {0xc0001e20c0, 0x0, 0x190, 0xc0001e2100, 0xc0001e2120, 0x0, 0x0, {0x0, 0x0}, ...})
	/Users/test/go/pkg/mod/github.com/ncruces/zenity@v0.8.8/progress_windows.go:198 +0x95b fp=0xc0004d9eb0 sp=0xc0004d9940 pc=0x709dfb
github.com/ncruces/zenity.progress.func1()
	/Users/test/go/pkg/mod/github.com/ncruces/zenity@v0.8.8/progress_windows.go:38 +0x58 fp=0xc0004d9fe0 sp=0xc0004d9eb0 pc=0x7090d8
runtime.goexit()
	/opt/homebrew/Cellar/go/1.18.3/libexec/src/runtime/asm_amd64.s:1571 +0x1 fp=0xc0004d9fe8 sp=0xc0004d9fe0 pc=0x41a121
created by github.com/ncruces/zenity.progress
	/Users/test/go/pkg/mod/github.com/ncruces/zenity@v0.8.8/progress_windows.go:37 +0x26e

There is similar crash and solution, plz see ref here

I think a possible solution maybe here for zenity@0.8.8, hope helps:

diff --git a/internal/win/kernel32.go b/internal/win/kernel32.go
index 7f44fe7..aea9658 100644
--- a/internal/win/kernel32.go
+++ b/internal/win/kernel32.go
@@ -45,3 +45,5 @@ func GetSystemDirectory() (string, error) { return windows.GetSystemDirectory()
 //sys GetConsoleWindow() (ret HWND) = kernel32.GetConsoleWindow
 //sys GetModuleHandle(moduleName *uint16) (ret Handle, err error) = kernel32.GetModuleHandleW
 //sys ReleaseActCtx(actCtx Handle) = kernel32.ReleaseActCtx
+//sys GlobalAlloc(flags uint32, dwBytes uintptr) (ret uintptr) = kernel32.GlobalAlloc
+//sys GlobalFree(dwBytes uintptr) (ret uintptr) = kernel32.GlobalFree
diff --git a/internal/win/user32.go b/internal/win/user32.go
index bf52b0a..c893979 100644
--- a/internal/win/user32.go
+++ b/internal/win/user32.go
@@ -303,20 +303,24 @@ func MessageLoop(wnd HWND) error {
 	isDialogMessage := procIsDialogMessageW.Addr()
 
 	for {
-		var msg MSG
-		s, _, err := syscall.Syscall6(getMessage, 4, uintptr(unsafe.Pointer(&msg)), 0, 0, 0, 0, 0)
+		// var msg MSG
+		msg := (*MSG)(unsafe.Pointer(GlobalAlloc(0, unsafe.Sizeof(MSG{}))))
+		s, _, err := syscall.Syscall6(getMessage, 4, uintptr(unsafe.Pointer(msg)), 0, 0, 0, 0, 0)
 		if int32(s) == -1 {
+			GlobalFree(uintptr(unsafe.Pointer(msg)))
 			return err
 		}
 		if s == 0 {
+			GlobalFree(uintptr(unsafe.Pointer(msg)))
 			return nil
 		}
 
-		s, _, _ = syscall.Syscall(isDialogMessage, 2, uintptr(wnd), uintptr(unsafe.Pointer(&msg)), 0)
+		s, _, _ = syscall.Syscall(isDialogMessage, 2, uintptr(wnd), uintptr(unsafe.Pointer(msg)), 0)
 		if s == 0 {
-			syscall.Syscall(translateMessage, 1, uintptr(unsafe.Pointer(&msg)), 0, 0)
-			syscall.Syscall(dispatchMessage, 1, uintptr(unsafe.Pointer(&msg)), 0, 0)
+			syscall.Syscall(translateMessage, 1, uintptr(unsafe.Pointer(msg)), 0, 0)
+			syscall.Syscall(dispatchMessage, 1, uintptr(unsafe.Pointer(msg)), 0, 0)
 		}
+		GlobalFree(uintptr(unsafe.Pointer(msg)))
 	}
 }
 
diff --git a/internal/win/zsyscall_windows.go b/internal/win/zsyscall_windows.go
index d28d2ae..69dbd2f 100644
--- a/internal/win/zsyscall_windows.go
+++ b/internal/win/zsyscall_windows.go
@@ -61,6 +61,8 @@ var (
 	procGenerateConsoleCtrlEvent     = modkernel32.NewProc("GenerateConsoleCtrlEvent")
 	procGetConsoleWindow             = modkernel32.NewProc("GetConsoleWindow")
 	procGetModuleHandleW             = modkernel32.NewProc("GetModuleHandleW")
+	procGlobalAlloc                  = modkernel32.NewProc("GlobalAlloc")
+	procGlobalFree                   = modkernel32.NewProc("GlobalFree")
 	procReleaseActCtx                = modkernel32.NewProc("ReleaseActCtx")
 	procCoCreateInstance             = modole32.NewProc("CoCreateInstance")
 	procSHBrowseForFolder            = modshell32.NewProc("SHBrowseForFolder")
@@ -203,6 +205,18 @@ func GetModuleHandle(moduleName *uint16) (ret Handle, err error) {
 	return
 }
 
+func GlobalAlloc(flags uint32, dwBytes uintptr) (ret uintptr) {
+	r0, _, _ := syscall.Syscall(procGlobalAlloc.Addr(), 2, uintptr(flags), uintptr(dwBytes), 0)
+	ret = uintptr(r0)
+	return
+}
+
+func GlobalFree(dwBytes uintptr) (ret uintptr) {
+	r0, _, _ := syscall.Syscall(procGlobalFree.Addr(), 1, uintptr(dwBytes), 0, 0)
+	ret = uintptr(r0)
+	return
+}
+
 func ReleaseActCtx(actCtx Handle) {
 	syscall.Syscall(procReleaseActCtx.Addr(), 1, uintptr(actCtx), 0, 0)
 	return

Thanks for the report and especially the code pointers.

This is scary, because current usage follows unsafe rules.
The explanation in github.com/lxn/walk/pull/493 is beyond my understanding, TBH, but it seems zenity is the nth project affected by this (also Wails, go-sciter, winc), so I'll just follow the herd.

I'll apply a slightly different patch to avoid allocating and deallocating memory for every loop.

Can you test the fix, or shall I make a release?

Tested, it works, no crashes~