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

@@ -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,
+#ifdef HAVE_EXCHNDL
-                        G_SPAWN_SEARCH_PATH | G_SPAWN_STDERR_TO_DEV_NULL | G_SPAWN_STDOUT_TO_DEV_NULL,
+          /* On Win32, the trace has already been processed by ExcHnl
-                        NULL, NULL, NULL, NULL, &exit_status, NULL);
+           * and is waiting for us in a text file.
-          eek_handled = TRUE;
+           * 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 */
 

app/main.c

@@ -36,11 +36,6 @@
 #ifdef G_OS_WIN32
 #include <io.h> /* get_osfhandle */
 
-#ifdef HAVE_EXCHNDL
-#include <time.h>
-#include <exchndl.h>
-#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);
 

app/signals.c

@@ -30,22 +30,68 @@
 #include "errors.h"
 #include "signals.h"
 
+#ifdef HAVE_EXCHNDL
+#include <windows.h>
+#include <time.h>
+#include <exchndl.h>
+
+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
+#ifdef G_OS_WIN32
-  /* No use catching these on Win32, the user won't get any stack
+  /* Use Dr. Mingw (dumps backtrace on crash) if it is available. Do
-   * trace from glib anyhow. It's better to let Windows inform about
+   * nothing otherwise on Win32.
-   * the program error, and offer debugging (if the user has installed
+   * The user won't get any stack trace from glib anyhow.
-   * MSVC or some other compiler that knows how to install itself as a
+   * 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 */

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__ */

app/widgets/gimpcriticaldialog.c

@@ -31,6 +31,15 @@
 #include <glib.h>
 #include <gtk/gtk.h>
 
+#ifdef PLATFORM_OSX
+#import <Cocoa/Cocoa.h>
+#endif
+
+#ifdef G_OS_WIN32
+#undef DATADIR
+#include <windows.h>
+#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_finalize (GObject     *object);
-static void    gimp_critical_dialog_response (GtkDialog  *dialog,
+static void    gimp_critical_dialog_response (GtkDialog   *dialog,
-                                              gint        response_id);
+                                              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 (),
+          browser_open_url (url, NULL);
-                        url,
-                        gtk_get_current_event_time(),
-                        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