From 4e5a5dbb87e36053c4ffbdd3d0208fcf41cc1a0e Mon Sep 17 00:00:00 2001 From: Jehan Date: Sat, 27 Jan 2018 16:43:43 +0100 Subject: [PATCH] app: make the backtrace GUI actually work on Win32. It was previously untested, hence as expected needed fixes. First I add our own exception handler using Win32 API SetUnhandledExceptionFilter(). Second, I reorder things so that ExcHndlInit() is run after this setter, since they will be executed as a FILO and we need backtraces to be generated before our separate GUI runs. Last I run the backtrace GUI as async. No need to keep the main GIMP waiting since the traces have already been generated into a separate file. Also replace gtk_show_uri() by the implementation taken straight from our web-browser plug-in, since apparently gtk_show_uri() doesn't work in Windows (and probably not macOS either since I see we have a separate implementation for this platform as well). I would like to be able to use the PDB but can't because this code needs to be usable both within the main process and into a separate tool process. Ideally, this should just be a utils function which could be included without a problem. --- app/errors.c | 33 +++++++-- app/main.c | 38 +--------- app/signals.c | 89 +++++++++++++++++++--- app/signals.h | 2 +- app/widgets/gimpcriticaldialog.c | 123 +++++++++++++++++++++++++++++-- 5 files changed, 224 insertions(+), 61 deletions(-) diff --git a/app/errors.c b/app/errors.c index 693a400c79..f4c0549f22 100644 --- a/app/errors.c +++ b/app/errors.c @@ -284,14 +284,18 @@ gimp_eek (const gchar *reason, #ifndef GIMP_CONSOLE_COMPILATION if (generate_backtrace && ! the_errors_gimp->no_interface) { - /* If enabled (it is disabled by default), the GUI preference + /* If GUI backtrace enabled (it is disabled by default), it * takes precedence over the command line argument. */ - gchar *args[7] = { "gimpdebug-2.0", full_prog_name, NULL, +#ifdef G_OS_WIN32 + const gchar *gimpdebug = "gimpdebug-2.0.exe"; +#else + const gchar *gimpdebug = "gimpdebug-2.0"; +#endif + gchar *args[7] = { (gchar *) gimpdebug, full_prog_name, NULL, (gchar *) reason, (gchar *) message, backtrace_file, NULL }; gchar pid[16]; - gint exit_status; g_snprintf (pid, 16, "%u", (guint) getpid ()); args[2] = pid; @@ -302,10 +306,25 @@ gimp_eek (const gchar *reason, * to keep GIMP up long enough for the debugger to get its * trace. */ - g_spawn_sync (NULL, args, NULL, - G_SPAWN_SEARCH_PATH | G_SPAWN_STDERR_TO_DEV_NULL | G_SPAWN_STDOUT_TO_DEV_NULL, - NULL, NULL, NULL, NULL, &exit_status, NULL); - eek_handled = TRUE; +#ifdef HAVE_EXCHNDL + /* On Win32, the trace has already been processed by ExcHnl + * and is waiting for us in a text file. + * We just want to spawn the trace GUI and exit GIMP directly. + */ + if (g_file_test (backtrace_file, G_FILE_TEST_IS_REGULAR) && + g_spawn_async (NULL, args, NULL, + G_SPAWN_SEARCH_PATH | G_SPAWN_STDERR_TO_DEV_NULL | G_SPAWN_STDOUT_TO_DEV_NULL, + NULL, NULL, NULL, NULL)) +#else + /* On Unix machines, the spawned process will attach to the + * main GIMP process and will generate the backtrace. So we + * run it as a synced process. + */ + if (g_spawn_sync (NULL, args, NULL, + G_SPAWN_SEARCH_PATH | G_SPAWN_STDERR_TO_DEV_NULL | G_SPAWN_STDOUT_TO_DEV_NULL, + NULL, NULL, NULL, NULL, NULL, NULL)) +#endif + eek_handled = TRUE; } #endif /* !GIMP_CONSOLE_COMPILATION */ diff --git a/app/main.c b/app/main.c index 7b6816754f..a25ef51d2f 100644 --- a/app/main.c +++ b/app/main.c @@ -36,11 +36,6 @@ #ifdef G_OS_WIN32 #include /* get_osfhandle */ -#ifdef HAVE_EXCHNDL -#include -#include -#endif - #endif /* G_OS_WIN32 */ #ifndef GIMP_CONSOLE_COMPILATION @@ -318,6 +313,9 @@ main (int argc, argv = __argv; #endif + /* Start signal handlers early. */ + gimp_init_signal_handlers (&backtrace_file); + #ifdef G_OS_WIN32 /* Reduce risks */ { @@ -364,34 +362,6 @@ main (int argc, g_free (bin_dir); } -#ifdef HAVE_EXCHNDL - /* Use Dr. Mingw (dumps backtrace on crash) if it is available. */ - { - time_t t; - gchar *filename; - gchar *dir; - - /* This has to be the non-roaming directory (i.e., the local - directory) as backtraces correspond to the binaries on this - system. */ - dir = g_build_filename (g_get_user_data_dir (), - GIMPDIR, GIMP_USER_VERSION, "CrashLog", - NULL); - /* Ensure the path exists. */ - g_mkdir_with_parents (dir, 0700); - - time (&t); - filename = g_strdup_printf ("%s-crash-%" G_GUINT64_FORMAT ".txt", - g_get_prgname(), t); - backtrace_file = g_build_filename (dir, filename, NULL); - g_free (filename); - g_free (dir); - - ExcHndlInit (); - ExcHndlSetLogFileNameA (backtrace_file); - } -#endif - #ifndef _WIN64 { typedef BOOL (WINAPI *t_SetProcessDEPPolicy) (DWORD dwFlags); @@ -538,8 +508,6 @@ main (int argc, if (abort_message) app_abort (no_interface, abort_message); - gimp_init_signal_handlers (); - if (system_gimprc) system_gimprc_file = g_file_new_for_commandline_arg (system_gimprc); diff --git a/app/signals.c b/app/signals.c index 76d5d4a5b3..c70748ca7d 100644 --- a/app/signals.c +++ b/app/signals.c @@ -30,22 +30,68 @@ #include "errors.h" #include "signals.h" +#ifdef HAVE_EXCHNDL +#include +#include +#include + +static LPTOP_LEVEL_EXCEPTION_FILTER g_prevExceptionFilter = NULL; + +static LONG WINAPI gimp_sigfatal_handler (PEXCEPTION_POINTERS pExceptionInfo); + +#else + +static void gimp_sigfatal_handler (gint sig_num) G_GNUC_NORETURN; -#ifndef G_OS_WIN32 -static void gimp_sigfatal_handler (gint sig_num) G_GNUC_NORETURN; #endif void -gimp_init_signal_handlers (void) +gimp_init_signal_handlers (gchar **backtrace_file) { -#ifndef G_OS_WIN32 - /* No use catching these on Win32, the user won't get any stack - * trace from glib anyhow. It's better to let Windows inform about - * the program error, and offer debugging (if the user has installed - * MSVC or some other compiler that knows how to install itself as a +#ifdef G_OS_WIN32 + /* Use Dr. Mingw (dumps backtrace on crash) if it is available. Do + * nothing otherwise on Win32. + * The user won't get any stack trace from glib anyhow. + * Without Dr. MinGW, It's better to let Windows inform about the + * program error, and offer debugging (if the user has installed MSVC + * or some other compiler that knows how to install itself as a * handler for program errors). */ +#ifdef HAVE_EXCHNDL + time_t t; + gchar *filename; + gchar *dir; + + /* Order is very important here. We need to add our signal handler + * first, then run ExcHndlInit() which will add its own handler, so + * that ExcHnl's handler runs first since that's in FILO order. + */ + if (! g_prevExceptionFilter) + g_prevExceptionFilter = SetUnhandledExceptionFilter (gimp_sigfatal_handler); + + /* This has to be the non-roaming directory (i.e., the local + directory) as backtraces correspond to the binaries on this + system. */ + dir = g_build_filename (g_get_user_data_dir (), + GIMPDIR, GIMP_USER_VERSION, "CrashLog", + NULL); + /* Ensure the path exists. */ + g_mkdir_with_parents (dir, 0700); + + time (&t); + filename = g_strdup_printf ("%s-crash-%" G_GUINT64_FORMAT ".txt", + g_get_prgname(), t); + *backtrace_file = g_build_filename (dir, filename, NULL); + g_free (filename); + g_free (dir); + + ExcHndlInit (); + ExcHndlSetLogFileNameA (*backtrace_file); + +#endif /* HAVE_EXCHNDL */ + +#else /* Handle fatal signals */ @@ -67,11 +113,32 @@ gimp_init_signal_handlers (void) /* Restart syscalls on SIGCHLD */ gimp_signal_private (SIGCHLD, SIG_DFL, SA_RESTART); -#endif /* ! G_OS_WIN32 */ +#endif /* G_OS_WIN32 */ } -#ifndef G_OS_WIN32 +#ifdef G_OS_WIN32 + +#ifdef HAVE_EXCHNDL +static LONG WINAPI +gimp_sigfatal_handler (PEXCEPTION_POINTERS pExceptionInfo) +{ + /* Just in case, so that we don't loop or anything similar, just + * re-establish previous handler. + */ + SetUnhandledExceptionFilter (g_prevExceptionFilter); + + /* Now process the exception. */ + gimp_fatal_error ("unhandled exception"); + + if (g_prevExceptionFilter && g_prevExceptionFilter != gimp_sigfatal_handler) + return g_prevExceptionFilter (pExceptionInfo); + else + return EXCEPTION_CONTINUE_SEARCH; +} +#endif + +#else /* gimp core signal handler for fatal signals */ @@ -97,4 +164,4 @@ gimp_sigfatal_handler (gint sig_num) } } -#endif /* ! G_OS_WIN32 */ +#endif /* G_OS_WIN32 */ diff --git a/app/signals.h b/app/signals.h index c846f961ae..97409f27ea 100644 --- a/app/signals.h +++ b/app/signals.h @@ -23,7 +23,7 @@ #endif -void gimp_init_signal_handlers (void); +void gimp_init_signal_handlers (gchar **backtrace_file); #endif /* __SIGNALS_H__ */ diff --git a/app/widgets/gimpcriticaldialog.c b/app/widgets/gimpcriticaldialog.c index a046d2ad96..c5f9edd353 100644 --- a/app/widgets/gimpcriticaldialog.c +++ b/app/widgets/gimpcriticaldialog.c @@ -31,6 +31,15 @@ #include #include +#ifdef PLATFORM_OSX +#import +#endif + +#ifdef G_OS_WIN32 +#undef DATADIR +#include +#endif + #include "gimpcriticaldialog.h" #include "version.h" @@ -44,9 +53,12 @@ typedef struct _GimpCriticalDialog GimpCriticalDialog; #define GIMP_CRITICAL_RESPONSE_RESTART 3 -static void gimp_critical_dialog_finalize (GObject *object); -static void gimp_critical_dialog_response (GtkDialog *dialog, - gint response_id); +static void gimp_critical_dialog_finalize (GObject *object); +static void gimp_critical_dialog_response (GtkDialog *dialog, + gint response_id); + +static gboolean browser_open_url (const gchar *url, + GError **error); G_DEFINE_TYPE (GimpCriticalDialog, gimp_critical_dialog, GTK_TYPE_DIALOG) @@ -166,6 +178,102 @@ gimp_critical_dialog_finalize (GObject *object) G_OBJECT_CLASS (parent_class)->finalize (object); } + +/* XXX This is taken straight from plug-ins/common/web-browser.c + * This really sucks but this class also needs to be called by + * tools/gimpdebug.c as a separate process and therefore cannot make use + * of the PDB. Anyway shouldn't we just move this as a utils function? + * Why does such basic feature as opening a URL in a cross-platform way + * need to be a plug-in? + */ +static gboolean +browser_open_url (const gchar *url, + GError **error) +{ +#ifdef G_OS_WIN32 + + HINSTANCE hinst = ShellExecute (GetDesktopWindow(), + "open", url, NULL, NULL, SW_SHOW); + + if ((gint) hinst <= 32) + { + const gchar *err; + + switch ((gint) hinst) + { + case 0 : + err = _("The operating system is out of memory or resources."); + break; + case ERROR_FILE_NOT_FOUND : + err = _("The specified file was not found."); + break; + case ERROR_PATH_NOT_FOUND : + err = _("The specified path was not found."); + break; + case ERROR_BAD_FORMAT : + err = _("The .exe file is invalid (non-Microsoft Win32 .exe or error in .exe image)."); + break; + case SE_ERR_ACCESSDENIED : + err = _("The operating system denied access to the specified file."); + break; + case SE_ERR_ASSOCINCOMPLETE : + err = _("The file name association is incomplete or invalid."); + break; + case SE_ERR_DDEBUSY : + err = _("DDE transaction busy"); + break; + case SE_ERR_DDEFAIL : + err = _("The DDE transaction failed."); + break; + case SE_ERR_DDETIMEOUT : + err = _("The DDE transaction timed out."); + break; + case SE_ERR_DLLNOTFOUND : + err = _("The specified DLL was not found."); + break; + case SE_ERR_NOASSOC : + err = _("There is no application associated with the given file name extension."); + break; + case SE_ERR_OOM : + err = _("There was not enough memory to complete the operation."); + break; + case SE_ERR_SHARE: + err = _("A sharing violation occurred."); + break; + default : + err = _("Unknown Microsoft Windows error."); + } + + g_set_error (error, 0, 0, _("Failed to open '%s': %s"), url, err); + + return FALSE; + } + + return TRUE; + +#elif defined(PLATFORM_OSX) + + NSURL *ns_url; + gboolean retval; + + @autoreleasepool + { + ns_url = [NSURL URLWithString: [NSString stringWithUTF8String: url]]; + retval = [[NSWorkspace sharedWorkspace] openURL: ns_url]; + } + + return retval; + +#else + + return gtk_show_uri (gdk_screen_get_default (), + url, + gtk_get_current_event_time(), + error); + +#endif +} + static void gimp_critical_dialog_response (GtkDialog *dialog, gint response_id) @@ -207,10 +315,7 @@ gimp_critical_dialog_response (GtkDialog *dialog, * much time digging into it. */ url = "https://bugzilla.gnome.org/enter_bug.cgi?product=GIMP"; - gtk_show_uri (gdk_screen_get_default (), - url, - gtk_get_current_event_time(), - NULL); + browser_open_url (url, NULL); } break; case GIMP_CRITICAL_RESPONSE_RESTART: @@ -218,6 +323,10 @@ gimp_critical_dialog_response (GtkDialog *dialog, gchar *args[2] = { critical->program , NULL }; #ifndef G_OS_WIN32 + /* It is unneeded to kill the process on Win32. This was run + * as an async call and the main process should already be + * dead by now. + */ if (critical->pid > 0) kill ((pid_t ) critical->pid, SIGINT); #endif