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

form: don't use Go heap for MSG #493

Merged
merged 1 commit into from
Apr 29, 2019
Merged

form: don't use Go heap for MSG #493

merged 1 commit into from
Apr 29, 2019

Conversation

zx2c4
Copy link
Contributor

@zx2c4 zx2c4 commented Apr 27, 2019

It's not entirely clear to me what's going on here, and I'm only
half-certain that this fixes the issue, due to it being somewhat hard to
reproduce. However, here's what I suspect is going on. Because we're
passing this as a pointer to various levels of function calls that might
store a reference to the pointer, Go allocates the variable on the heap.
Later, various closures are allocated on the same heap, during which
time some global Go lock is taken and W^X page permissions are twiddled
with. The Go locking then doesn't effect functions that are in win32api
calls, and at the critical moment, the variable no longer has writable
page permissions. While this might point to deeper Go runtime issues, we
just work around this here by allocating the variable using GlobalAlloc.
At the very least, I haven't been able to reproduce the bug after
applying this patch.

Fixes: #483

It's not entirely clear to me what's going on here, and I'm only
half-certain that this fixes the issue, due to it being somewhat hard to
reproduce. However, here's what I suspect is going on. Because we're
passing this as a pointer to various levels of function calls that might
store a reference to the pointer, Go allocates the variable on the heap.
Later, various closures are allocated on the same heap, during which
time some global Go lock is taken and W^X page permissions are twiddled
with. The Go locking then doesn't effect functions that are in win32api
calls, and at the critical moment, the variable no longer has writable
page permissions. While this might point to deeper Go runtime issues, we
just work around this here by allocating the variable using GlobalAlloc.
At the very least, I haven't been able to reproduce the bug after
applying this patch.

Fixes: lxn#483
Signed-off-by: Jason A. Donenfeld <[email protected]>
@zx2c4
Copy link
Contributor Author

zx2c4 commented Apr 27, 2019

I'm not 100% that this actually works or that the analysis is correct, so please test it for a bit and see if you can reproduce the crashes.

@zx2c4
Copy link
Contributor Author

zx2c4 commented Apr 29, 2019

@StephanVerbeeck - can you verify that this fixes the issue for you?

@lxn lxn merged commit 46cab9d into lxn:master Apr 29, 2019
@StephanVerbeeck
Copy link

@StephanVerbeeck - can you verify that this fixes the issue for you?

With pleasure, but it will take a few days as the errors occurrence is infrequent and with irregular timing.

@StephanVerbeeck
Copy link

hmmmmmmm . . . . issue #491 seems to be fixed by this :-) :-O

@tmm1
Copy link
Contributor

tmm1 commented May 10, 2019

Thanks for getting to the bottom of this!

Later, various closures are allocated on the same heap, during which
time some global Go lock is taken and W^X page permissions are twiddled
with.

Why does the go runtime tweak page permissions?

@StephanVerbeeck
Copy link

@StephanVerbeeck - can you verify that this fixes the issue for you?

The crash did not occur since (there were some other but not related to the dialog message)
I have installed a crash-stackdump-logger in my app but some are not caught because they happen on pure runtime (not called from my main function and none of the functions on the stack dump are mine). I do not know of a method to log those (my accounting app has an option in the settings screen where the user can send the stackdump logs over to me by email but that then only works for code that is called from my main function).

@zhaoya881010
Copy link

Exception 0xc0000005 0x0 0x1749111c 0x77e47594
PC=0x77e47594
signal arrived during external code execution

syscall.Syscall6(0x7598a5b0, 0x4, 0x27066f8, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
C:/Go/src/runtime/syscall_windows.go:201 +0xbb
GoProxy/vendor/github.com/lxn/win.GetMessage(0x27066f8, 0x0, 0x0, 0x0, 0x1)
D:/svn_code/src/GoProxy/vendor/github.com/lxn/win/user32.go:2564 +0x8a
GoProxy/vendor/github.com/lxn/walk.(*FormBase).mainLoop(0x2eb9a000, 0x0)
D:/svn_code/src/GoProxy/vendor/github.com/lxn/walk/mainloop_default.go:20 +0x95
GoProxy/vendor/github.com/lxn/walk.(*FormBase).Run(0x2eb9a000, 0x0)
D:/svn_code/src/GoProxy/vendor/github.com/lxn/walk/form.go:373 +0xa3
main.main()

@zhaoya881010
Copy link

It seems to have something to do with the operating system

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sporadic Crash at form.go#432
5 participants