Add error dialog for critical errors on Windows #54836
Conversation
giordano
added
system:windows
topolarity
force-pushed
the
ct/more-fprint
branch
from
9f57800 to
37ce01c
Compare
topolarity
force-pushed
the
ct/windows-alert
branch
from
44c0470 to
66c5a55
Compare
This can be very useful when your Julia-built library hits an unexpected exception and causes a GUI-based application to crash with no error message.
topolarity
force-pushed
the
ct/windows-alert
branch
from
66c5a55 to
7768244
Compare
topolarity
force-pushed
the
ct/windows-alert
branch
from
7768244 to
ca4254f
Compare
| event_source, EVENTLOG_ERROR_TYPE, /* category */ 0, /* event_id */ (DWORD)0xE0000000L, | ||
| /* user_sid */ NULL, /* n_strings */ 1, /* data_size */ 0, strings, /* data */ NULL | ||
| ); | ||
| MessageBoxW(NULL, /* message */ L"error: libjulia received a fatal signal.\n\n" |
There was a problem hiding this comment.
I know very little about Windows, so my apologies if these questions are dumb, but: 1. Is this call async, or does it block?
2. Is this really safe to call here? In particular,
3. I am slightly worried about situations I experienced in an entirely different context were broadly similar code could lead to hundreds of dialog boxes spamming a user (when code that launched a ton of subprocesses to process data had them all fail simultaneously -- e.g. because a network connection dropped).
Anyway, none of this is meant as argument against this PR (which to me, as a non-Windows user, seems quite useful). Feel free to ignore me :-)
There was a problem hiding this comment.
On 1: it blocks the thread.
|
Probably also needs to detect scenarios where no windows desktop is present? SSH, PowerShell remoting, running as a service etc. Somehow making the dialog box optional seems like a good idea to me. |
| @@ -278,59 +298,88 @@ LONG WINAPI jl_exception_handler(struct _EXCEPTION_POINTERS *ExceptionInfo) | |||
| break; | |||
| } | |||
| } | |||
| ios_t full_error, summary; | |||
| ios_mem(&full_error, 1024); | |||
There was a problem hiding this comment.
calling malloc unconditionally here is a bit wasteful, since we will call it on demand when actually needed
| ios_mem(&full_error, 1024); | |
| ios_mem(&full_error, 0); |
| /* Fast-path empty strings, as MultiByteToWideChar() returns zero for them. */ | ||
| if (str[0] == '\0') { | ||
| wchar_t *wstr = (wchar_t *)malloc(sizeof(wchar_t)); | ||
| wstr[0] = L'\0'; | ||
| return wstr; | ||
| } | ||
| size_t len = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0); | ||
| if (!len) | ||
| return NULL; | ||
| wchar_t *wstr = (wchar_t *)malloc(len * sizeof(wchar_t)); | ||
| if (!wstr || !MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, len)) | ||
| return NULL; | ||
| return wstr; |
There was a problem hiding this comment.
| /* Fast-path empty strings, as MultiByteToWideChar() returns zero for them. */ | |
| if (str[0] == '\0') { | |
| wchar_t *wstr = (wchar_t *)malloc(sizeof(wchar_t)); | |
| wstr[0] = L'\0'; | |
| return wstr; | |
| } | |
| size_t len = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0); | |
| if (!len) | |
| return NULL; | |
| wchar_t *wstr = (wchar_t *)malloc(len * sizeof(wchar_t)); | |
| if (!wstr || !MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, len)) | |
| return NULL; | |
| return wstr; | |
| ssize_t wlen = uv_wtf8_length_as_utf16(str); | |
| wchar_t *wstr = (wchar_t *)malloc_s(sizeof(wchar_t) * wlen); | |
| uv_wtf8_to_utf16(str, wstr, wlen); | |
| return wstr; |
This is a more reliable (and concise) version of MultiByteToWideChar now provided by libuv
| if (event_source != INVALID_HANDLE_VALUE) { | ||
| ios_putc('\0', &full_error); | ||
| const wchar_t *strings[] = { | ||
| ios_utf8_to_wchar(full_error.buf), |
This change adds a small dialog for fatal errors on Windows:
with a corresponding entry in the
Applicationlog (in Event Viewer)This is intended primarily for users of PackageCompiler.jl, who might be using Julia in a GUI application.
If a Julia-compiled library fails in a GUI application, users currently they have no good way to know why/how their application crashed if, e.g., an exception was accidentally left unhandled in their Julia code.
Depends on #54835
TODO: Should this be a
jl_optionsflag to determine whether to show the GUI? Should we log to the event log unconditionally? Is there any way that we can automatically detect whetherstderris connected to a terminal and user-visible?