Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syscall: unexplained crashes when using win32api (PostMessage + Window functions) #50872

Closed
SeanTolstoyevski opened this issue Jan 27, 2022 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@SeanTolstoyevski
Copy link

What version of Go are you using (go version)?

go version go1.17.3 windows/amd64

windows 10 64bit

Does this issue reproduce with the latest release?

I don't know, I haven't tried it with beta versions or older versions.

What operating system and processor architecture are you using (go env)?

go env Output

set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\dl\AppData\Local\go-build
set GOENV=C:\Users\dl\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\dl\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\dl\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.17.3
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\dl\AppData\Local\Temp\go-build2985149018=/tmp/go-build -gno-record-gcc-switches

What did you do?

After long test I decided to share this. In this way, maybe a solution can be found, or this is the cause of an error in Golang's runtime.

Here I am summarizing the issue so that everyone can understand what I want to do:
Let's create a simple Win32api application. I will share an example program below.
We don't use cgo. We will use syscall.

The Win32api function GetMessage blocks the program if there are no messages in the event queue. So if no event is created, the program waits forever for an event to occur.
So I'm sending messages to the queue with PostMessage. I do this with time.Ticker.
Now we have prevented the program from being blocked forever. We can receive the events we send with PostMessage from the queue.
As a note, PostMessage runs another goroutine. Using runtime.LockOSThread does not prevent the program from crashing.

The program can run without anything for a long time. Sometimes it crashes with no errors. Sometimes it works normally for a very long time, sometimes it crashes immediately. Finally, after a certain time, the program stops responding.
This made me think about the possibility of race condition. But there is no such thing.
I wrote a duplicate of the same code in C++. Win32api shows no problems. The program never crashes. I know that C++ and Golang are not the same thing. I just wanted to understand where the problem is.
So the program not crashing in C++ makes me think that Golang is doing something wrong for win32api in runtime.
My guess is that he is moving the runtime goroutines to different threads or dealing with a similar golang thing. Some win32api functions are sensitive to the called thread. In other words, the thread that the goroutine is connected to should not change. Maybe GC is deleting something important for win32api. I really don't know what's going on here.

I tried to keep this issue simple so anyone can generate it themselves but since win32api is a low level api it takes a long time to create something.

Those who want to confirm this example can run the program and wait. You can leave the program's window and switch to something else. Because sometimes it works right for too long for incomprehensible reasons.

I don't know how to test this in a simpler way.

Go code (go build app.go):

package main

import (
	"errors"
	"fmt"
	"syscall"
	"unsafe"

	"time"
	"runtime"
)

var (
	k32 = syscall.NewLazyDLL("kernel32.dll")

	procGetModuleHandle         = k32.NewProc("GetModuleHandleW")
)

var (
	u32 = syscall.NewLazyDLL("user32.dll")

	procRegisterClassEx            = u32.NewProc("RegisterClassExW")
	procUpdateWindow               = u32.NewProc("UpdateWindow")
	procCreateWindowEx             = u32.NewProc("CreateWindowExW")
	procDestroyWindow              = u32.NewProc("DestroyWindow")
	procDefWindowProc              = u32.NewProc("DefWindowProcW")
	procPostQuitMessage            = u32.NewProc("PostQuitMessage")
	procGetMessage                 = u32.NewProc("GetMessageW")
	procTranslateMessage           = u32.NewProc("TranslateMessage")
	procDispatchMessage            = u32.NewProc("DispatchMessageW")
	procPostMessage                = u32.NewProc("PostMessageW")
)


func RegisterClassEx(wndClassEx *WNDCLASSEX) ATOM {
	ret, _, _ := procRegisterClassEx.Call(uintptr(unsafe.Pointer(wndClassEx)))
	return ATOM(ret)
}


func UpdateWindow(hwnd HWND) bool {
	ret, _, _ := procUpdateWindow.Call(
		uintptr(hwnd))
	return ret != 0
}

func CreateWindowEx(exStyle uint, className, windowName *uint16,
	style uint, x, y, width, height int, parent HWND, menu HMENU,
	instance HINSTANCE, param unsafe.Pointer) HWND {
	ret, _, _ := procCreateWindowEx.Call(uintptr(exStyle), uintptr(unsafe.Pointer(className)),
		uintptr(unsafe.Pointer(windowName)),
		uintptr(style),
		uintptr(x),
		uintptr(y),
		uintptr(width),
		uintptr(height),
		uintptr(parent),
		uintptr(menu),
		uintptr(instance),
		uintptr(param))
	return HWND(ret)
}

func DestroyWindow(hwnd HWND) bool {
	ret, _, _ := procDestroyWindow.Call(
		uintptr(hwnd))
	return ret != 0
}

func DefWindowProc(hwnd HWND, msg uint32, wParam, lParam uintptr) uintptr {
	ret, _, _ := procDefWindowProc.Call(uintptr(hwnd), uintptr(msg), wParam, lParam)
	return ret
}

func PostMessage(hwnd HWND, msg uint32, wParam, lParam uintptr) bool {
	ret, _, _ := procPostMessage.Call(uintptr(hwnd), uintptr(msg), wParam, lParam)
	return ret != 0
}

func TranslateMessage(msg *MSG) bool {
	ret, _, _ := procTranslateMessage.Call(
		uintptr(unsafe.Pointer(msg)))
	return ret != 0
}

func DispatchMessage(msg *MSG) uintptr {
	ret, _, _ := procDispatchMessage.Call(
		uintptr(unsafe.Pointer(msg)))
	return ret
}

func GetMessage(msg *MSG, hwnd HWND, msgFilterMin, msgFilterMax uint32) int {
	ret, _, _ := procGetMessage.Call( uintptr(unsafe.Pointer(msg)), uintptr(hwnd), uintptr(msgFilterMin), uintptr(msgFilterMax))
	return int(ret)
}


func PostQuitMessage(exitCode int) {
	procPostQuitMessage.Call(
		uintptr(exitCode))
}

func GetModuleHandle(lpModuleName *uint16) HINSTANCE {
	ret, _, _ := procGetModuleHandle.Call(uintptr(unsafe.Pointer(lpModuleName)))
	return HINSTANCE(ret)
}

type (
	ATOM uint16

	DWORD uint32

	HANDLE uintptr

	HINSTANCE HANDLE

	HMODULE HANDLE

	HRESULT int32

	HWND HANDLE

	LPARAM uintptr

	HKL     HANDLE

	LPCVOID unsafe.Pointer

	LRESULT uintptr

	PVOID unsafe.Pointer

	QPC_TIME uint64

	ULONG_PTR uintptr

	WPARAM uintptr

	HCURSOR HANDLE

	HICON HANDLE

	HMENU HANDLE


	HHOOK HANDLE

	HRAWINPUT HANDLE

	COLORREF uint32

	HBITMAP HGDIOBJ

	HBRUSH HGDIOBJ

	HDC HANDLE

	HFONT HGDIOBJ

	HGDIOBJ HANDLE


	HPALETTE HGDIOBJ


	HRGN HGDIOBJ

)

// http://msdn.microsoft.com/en-us/library/windows/desktop/ms633577.aspx
type WNDCLASSEX struct {
	Size       uint32
	Style      uint32
	WndProc    uintptr
	ClsExtra   int32
	WndExtra   int32
	Instance   HINSTANCE
	Icon       HICON
	Cursor     HCURSOR
	Background HBRUSH
	MenuName   *uint16
	ClassName  *uint16
	IconSm     HICON
}

// http://msdn.microsoft.com/en-us/library/windows/desktop/dd162805.aspx
type POINT struct {
	X, Y int32
}

// http://msdn.microsoft.com/en-us/library/windows/desktop/ms644958.aspx
type MSG struct {
	Hwnd    HWND
	Message uint32
	WParam  uintptr
	LParam  uintptr
	Time    uint32
	Pt      POINT
}


const (
	TickerMSG uint32 = 0x10

	WM_USER                   = 1024

	WM_DESTROY                = 2

	WM_QUIT                   = 18

	WS_EX_APPWINDOW        = 0x00040000

	WS_VISIBLE          = 0x10000000

	COLOR_BACKGROUND              = 1

	WS_POPUP            = 0x80000000


	)

const CW_USEDEFAULT = ^0x7fffffff

type Window struct {
	wc                WNDCLASSEX
	msg               MSG
	die               chan struct{}
	ticker            *time.Ticker
	tickerMillisec    time.Duration
	tickerActive      bool
	handle            HWND
}

func NewWindow(title string, wi, he uint32) (*Window, error) {
	var w Window
	w.die = make(chan struct{}, 1)
	utf16ClassName := syscall.StringToUTF16Ptr("myWinApp")
	w.wc.Size = uint32(unsafe.Sizeof(w.wc))
	w.wc.Style = 0
	w.wc.WndProc = syscall.NewCallback(w.callback)
	w.wc.Background = (HBRUSH)(COLOR_BACKGROUND)
	w.wc.Instance = GetModuleHandle(nil)
	w.wc.ClassName = utf16ClassName
	if RegisterClassEx(&w.wc) == 0 {
		return nil, errors.New("RegisterClassEx error")
	}
	w.handle = CreateWindowEx(WS_EX_APPWINDOW, utf16ClassName, syscall.StringToUTF16Ptr(title), WS_POPUP | WS_VISIBLE, CW_USEDEFAULT, CW_USEDEFAULT, int(wi), int(he), 0, 0, GetModuleHandle(nil), nil)
	if w.handle == 0 {
		return nil, errors.New("CreateWindowEx error")
	}
	UpdateWindow(w.handle)
	return &w, nil
}

func (w *Window) callback(hwnd HWND, msg uint32, wParam, lParam uintptr) uintptr {
	if msg == WM_DESTROY || msg == WM_QUIT {
		PostQuitMessage(0)
		return 0
	}
	if msg == WM_USER + TickerMSG {
		return 0
	}
	return DefWindowProc(hwnd, msg, wParam, lParam)
}

func (w *Window) StartTicker(millisec uint32) {
	runtime.LockOSThread()
	defer runtime.UnlockOSThread()
	// don't start before main loop
	time.Sleep(500 * time.Millisecond)
	w.tickerMillisec = time.Duration(millisec) * time.Millisecond
	w.tickerActive = true
	w.ticker = time.NewTicker(w.tickerMillisec)
	defer w.ticker.Stop()
	for w.tickerActive {
		select {
		case <-w.ticker.C:
			if !PostMessage(w.handle, WM_USER + TickerMSG, 0, 0) {
				fmt.Println("Ticker error. Exiting.")
				w.tickerActive = false
				return
			}
		case <-w.die:
			w.tickerActive = false
			return
		}
	}
}

func (w *Window) Destroy() {
	DestroyWindow(w.handle)
}

func (w *Window) Update()  bool {
	b := GetMessage(&w.msg, 0, 0, 0) > 0
	if !b {
		return false
	}
	TranslateMessage(&w.msg)
	DispatchMessage(&w.msg)
	return true
}


func main() {
	w, err := NewWindow("test window from golang", 440, 440)
	if err != nil {
		fmt.Println(err)
		return
	}
	defer w.Destroy()
	go w.StartTicker(5) // every 5 milliseconds

	for w.Update() {
		
}

}

C++ (g++ app.cpp):

#include <iostream>
#include <windows.h>
#include <thread>
#include <chrono>


void ticker(HWND hwnd)
{
	using namespace std::chrono_literals;
	// don't run before mainloop
	std::this_thread::sleep_for(1000ms);
	bool r = true;
	while(r) {
		if(!PostMessage(hwnd, WM_USER+10, 0, 0)) {
			MessageBox(NULL, "PostMessage error!", "Error!", MB_ICONEXCLAMATION | MB_OK);
			r = false;
			break;
		}
		std::this_thread::sleep_for(5ms); // sleep 5ms and send message
	}
}

// forward decleration
LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam);

int main()
{
MSG msg          = {0};
	HWND handle;

	WNDCLASS wc      = {0}; 
	wc.lpfnWndProc   = WndProc;
	wc.hInstance     = GetModuleHandle(NULL);
	wc.hbrBackground = (HBRUSH)(COLOR_BACKGROUND);
	wc.lpszClassName = "minwindowsapp";
	if( !RegisterClass(&wc) )
		return 1;

	handle = CreateWindow(wc.lpszClassName, "Minimal Windows Application", WS_OVERLAPPEDWINDOW|WS_VISIBLE, 0,0, 640, 480, 0, 0, GetModuleHandle(NULL), NULL);
	if(handle == NULL) {
		return 2;
	}

	std::thread t1(ticker, handle);
	t1.detach();

	while(GetMessage(&msg, NULL, 0, 0 ) > 0) {
		TranslateMessage(&msg);
		DispatchMessage(&msg);
	}
	return 0;
}

LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
{
	switch(message) {
	case WM_CLOSE:
		PostQuitMessage(0);
	break;
	default:
		return DefWindowProc(hWnd, message, wParam, lParam);
	}
return 0;
}

What did you expect to see?

The program should not crash.
Ok, a program can crash anytime. But it should not crash sporadically, for no reason. I want to know the problem here.

What did you see instead?

I see a program crashing at unpredictable times.
Sometimes it works correctly for a very long time. Sometimes it crashes within 1-2 minutes.

I think the crash was caused by a component that started running in the runtime. Maybe gc is kicking in or something.

I am apologize for my English. It's not my native language. I hope the codes I shared describe the problem better.

@beoran
Copy link

beoran commented Jan 27, 2022

You need to use this:
https://github.com/faiface/mainthread

Otherwise there is a chance that you will not be on the main thread when the windows GUI code executes, which is not allowed. After startup, the Go runtime can switch threads transparently.

@SeanTolstoyevski
Copy link
Author

Hi @beoran ,

All this package does is call runtime.LockOSThread at startup.
It didn't work in the ticker function, but I'll try it in the main function.

@beoran
Copy link

beoran commented Jan 28, 2022

Please study that package in detail. Timing and functionality are crucial. Lockosthread must be called in an init function, and then you have to post callbacks on a channel to be sure that you are on the main thread. Once in the main function the go runtime might already have started another thread.

If you don't understand the details then by all means it is best you use that package and call the windows GUI API from that package's Run function.

@mknyszek
Copy link
Contributor

I haven't taken a close look, but yes, anything that requires running on the main thread (like most GUI-related things) requires some special care, and it's more subtle than you might think.

The precise steps are:

  1. LockOSThread in any init function.
  2. main will now be called in a goroutine locked to the main thread.
  3. Start a goroutine in main to do the rest of your program's work.
  4. Accept requests on main via a channel or some other mechanism to run code on that goroutine.

This is exactly what https://github.com/faiface/mainthread does, but it wraps it in an API.

Given that this is well-understood, I think that explains what could be going wrong with your program. If you have a reproducer that only relies on PostMessage and such, then that's another story.

Leaving this open for the moment as waiting for info.

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 28, 2022
@mknyszek mknyszek added this to the Backlog milestone Jan 28, 2022
@mknyszek mknyszek added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 28, 2022
@mknyszek mknyszek changed the title syscall: win32api: Unexplained crashes when using win32api (PostMessage) syscall: unexplained crashes when using win32api (PostMessage + Window functions) Jan 28, 2022
@SeanTolstoyevski
Copy link
Author

Hi,

in my case call runtime.LockOSThread in main function fixed the problem.
You can close.

@beoran
Copy link

beoran commented Jan 28, 2022

Although this issue was closed, perhaps it should be documented somewhere a bit better what running on the main thread is and when, why and how it should be done?

Perhaps in the lockosthread documentation?

@mknyszek
Copy link
Contributor

@beoran See #23112 and https://pkg.go.dev/runtime#LockOSThread. I'm not sure where else this should be documented, but if another place feels better to you, then let us know. Or you can send us a PR as well. :)

@beoran
Copy link

beoran commented Jan 29, 2022

It could be added there that it is necessary to lick and use the main thread for Windows and Osx GUI calls and that on Unix/Linux the thread that connects to X11 or Wayland needs to be locked and used for future calls.

As for a PR. I oppose CLAs for floss, so I cannot contribute to Go except by giving suggestions here. A DCO should be sufficient. ( http://esr.ibiblio.org/?p=8287) Once you allow people to contribute with a DCO, I definitely will join in.

@SeanTolstoyevski
Copy link
Author

Hi @mknyszek , @beoran ,

I don't think the documentation is missing, but a little more detail would be nice.

For example, why does calling LockOSThread inside the goroutine (see. StartTicker) not work, as I tried?
It would be good to know that.

As I understand it, the goroutine I am calling is supposed to prevent it from switching to another thread etc , but I seem to have misunderstood this.

As another form of question: Is LockOSThread only there to work in the main function?

@beoran
Copy link

beoran commented Jan 29, 2022

Lockosthread also works later on, but it may lock the goroutine to a non main thread from which you cannot call Windows GUI functions.

@golang golang locked and limited conversation to collaborators Jan 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants