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

runtime: Go should handle all Windows exceptions #8006

Closed
alexbrainman opened this issue May 16, 2014 · 16 comments
Closed

runtime: Go should handle all Windows exceptions #8006

alexbrainman opened this issue May 16, 2014 · 16 comments
Milestone

Comments

@alexbrainman
Copy link
Member

Sometimes Go programs crash without any stack trace / error message. Instead they defer
"exception handling" onto default system exception handler (Dr. Watson, Visual
Studio debugger or whatever else user installed).

From https://groups.google.com/d/msg/golang-nuts/PKdpkSTS1Rg/y69ABuLR_dcJ

>>>
So what happens is that the program tries to access address 0xfffff... and a dialog
appears offering to debug with VS. Doing this shows that this illegal instruction is the
call to std::cerr.
<<<

It didn't happen in the past. But since new exception handler 0a3722aa9092, it is now
possible. Our current handler handles only Go code, but any other exceptions are just
ignored.

Not sure how much information we can print in these situations, but we should try. And
we should stop "Dr. Watson to be invoked, because that requires user intervention
and is not acceptable in general.

Alex
@ianlancetaylor
Copy link
Member

Comment 1:

Labels changed: added repo-main, release-none, os-windows.

@davecheney
Copy link
Contributor

Comment 2:

Hi Alex,
I believe late in the 1.3 cycle you and Minux had some proposals for this. I've
tentatively marked this issue go-1.4, if you think this is not practical, please remove
the label.

Labels changed: added release-go1.4, removed release-none.

Status changed to Accepted.

@minux
Copy link
Member

minux commented Jul 16, 2014

Comment 3:

the problem of this is: VEH handler takes precedence over SEH handlers,
and it's natural for other windows code to register SEH handler and
then trigger an exception, which get delivered to our VEH handler.
The VEH handler doesn't know if the SEH is going to handle it (and it's
generally impossible to know, as one must actually invoke the SEH chain
to see if a given exception will be handled), so we ignore them and hope
for the best.
In the past, I think we used to handle all of them, but some windows dlls
are broken because they couldn't receive their exception.
I'd like to see an example that causes this problem before proceeding.
If the exception happens on OS threads managed by Go, then we should find
a way to handle it. If not, then I'm afraid we can't do anything here.

@alexbrainman
Copy link
Member Author

Comment 4:

I think Go executable should handle all unhandled exceptions. Like we did before. We
should print stack trace and exit with some error code. If we cannot print stack trace,
we should at least exit. At this moment, we defer to other system tools to exit our
process and that is unacceptable (some of those require user intervention and so).
My general idea is to split our current exception handler into two:
- first will decide if Go should handle exception, and handle it if possible (anything
that gets passed onto Go recover);
- second will just print stack trace and exit process.
We should call the second as late as we can on exception handlers chain. I think we
should be able to use one of those: AddVectoredExceptionHandler,
AddVectoredContinueHandler or SetUnhandledExceptionFilter to arrange that.
Alex

@rsc
Copy link
Contributor

rsc commented Sep 16, 2014

Comment 5:

I agree with Minux: can we get an example so that the discussion can be more concrete?
It's getting a bit too late in the release cycle to do anything.

@alexbrainman
Copy link
Member Author

Comment 6:

Apply this
diff --git a/src/runtime/syscall_windows_test.go b/src/runtime/syscall_windows_test.go
--- a/src/runtime/syscall_windows_test.go
+++ b/src/runtime/syscall_windows_test.go
@@ -488,3 +488,11 @@
        t.Fatalf("UnregisterClass failed: %v", err)
    }
 }
+
+func TestRaiseException(t *testing.T) {
+   const EXCEPTION_NONCONTINUABLE = 1
+   d := GetDLL(t, "kernel32.dll")
+   p := d.Proc("RaiseException")
+   p.Call(0xbad, EXCEPTION_NONCONTINUABLE, 0, 0)
+   t.Fatal("RaiseException should not return")
+}
to
hg id
234ba1c7ed3a+ tip  
and run it.
I have seen it happens on our builders too. I have no access to new windows builders,
but I suspect it is happening there too. For example,
http://build.golang.org/log/bfbbee3ca2be397eb0be04df49d55060b7a7b27b
>>>
...
go/build
cmd/go
        1 file(s) moved.
Build complete, duration 1h0m2.6278153s. Result: error: timed out after 1h0m0s
<<<<
Why did it take 1 hour to complete the build? What was it waiting for? Perhaps broken
program is still suspended waiting for user input there.
I have been waiting for dust to settle before building a fix for it. I will try to do it
now. If it has to wait till after 1.4, so be it.
Alex

@gopherbot
Copy link
Contributor

Comment 7:

CL https://golang.org/cl/146800043 mentions this issue.

@gopherbot
Copy link
Contributor

Comment 8:

CL https://golang.org/cl/138630043 mentions this issue.

@alexbrainman
Copy link
Member Author

Comment 9:

This issue was updated by revision f8474fa.

LGTM=adg, rsc
R=golang-codereviews, adg, rsc
CC=golang-codereviews
https://golang.org/cl/138630043

@alexbrainman
Copy link
Member Author

Comment 10:

This issue was updated by revision 2ed209e.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/146800043

@gopherbot
Copy link
Contributor

Comment 11:

CL https://golang.org/cl/145150043 mentions this issue.

@alexbrainman
Copy link
Member Author

Comment 12:

This issue was closed by revision 17a108b.

Status changed to Fixed.

@alexbrainman
Copy link
Member Author

Comment 13:

This issue was closed by revision 64736ac.

@alexbrainman
Copy link
Member Author

Comment 14:

revision 22318cd31d7d was an undo. So reopening tnhis issue.
Alex

Status changed to Accepted.

@gopherbot
Copy link
Contributor

Comment 15:

CL https://golang.org/cl/155360043 mentions this issue.

@alexbrainman
Copy link
Member Author

Comment 16:

This issue was closed by revision e9ecd4a.

Status changed to Fixed.

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/145150043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
That was complete failure - builders are broken,
but original cl worked fine on my system.
I will need access to builders
to test this change properly.

««« original CL description
runtime: handle all windows exception

Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/145150043
»»»

TBR=rsc
R=golang-codereviews
CC=golang-codereviews
https://golang.org/cl/154180043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
includes undo of 22318cd31d7d and also:
- always use SetUnhandledExceptionFilter on windows-386;
- crash when receive EXCEPTION_BREAKPOINT in exception handler.

Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/155360043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/145150043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
That was complete failure - builders are broken,
but original cl worked fine on my system.
I will need access to builders
to test this change properly.

««« original CL description
runtime: handle all windows exception

Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/145150043
»»»

TBR=rsc
R=golang-codereviews
CC=golang-codereviews
https://golang.org/cl/154180043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
includes undo of 22318cd31d7d and also:
- always use SetUnhandledExceptionFilter on windows-386;
- crash when receive EXCEPTION_BREAKPOINT in exception handler.

Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/155360043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/145150043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
That was complete failure - builders are broken,
but original cl worked fine on my system.
I will need access to builders
to test this change properly.

««« original CL description
runtime: handle all windows exception

Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/145150043
»»»

TBR=rsc
R=golang-codereviews
CC=golang-codereviews
https://golang.org/cl/154180043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
includes undo of 22318cd31d7d and also:
- always use SetUnhandledExceptionFilter on windows-386;
- crash when receive EXCEPTION_BREAKPOINT in exception handler.

Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/155360043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/145150043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
That was complete failure - builders are broken,
but original cl worked fine on my system.
I will need access to builders
to test this change properly.

««« original CL description
runtime: handle all windows exception

Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/145150043
»»»

TBR=rsc
R=golang-codereviews
CC=golang-codereviews
https://golang.org/cl/154180043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
includes undo of 22318cd31d7d and also:
- always use SetUnhandledExceptionFilter on windows-386;
- crash when receive EXCEPTION_BREAKPOINT in exception handler.

Fixes golang#8006.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/155360043
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants