Improved out of process crash handler (#5543)

* Add out of process crash handler for better crash handling.

src/CMakeLists.txt

@@ -9,6 +9,9 @@ IF (WITH_GUI)
 ENDIF (WITH_GUI)
 ADD_SUBDIRECTORY(providers)
 ADD_SUBDIRECTORY(crssync)
+IF(WIN32)
+  ADD_SUBDIRECTORY(crashhandler)
+ENDIF(WIN32)
 ADD_SUBDIRECTORY(test)
 
 IF (WITH_DESKTOP)

src/app/CMakeLists.txt

@@ -20,7 +20,6 @@ SET(QGIS_APP_SRCS
   qgsclipboard.cpp
   qgscustomization.cpp
   qgscustomprojectiondialog.cpp
-  qgscrashreport.cpp
   qgsdecorationitem.cpp
   qgsdecorationcopyright.cpp
   qgsdecorationcopyrightdialog.cpp
@@ -200,7 +199,6 @@ SET(QGIS_APP_SRCS
 
   qgssettingstree.cpp
   qgsvariantdelegate.cpp
-  qgscrashdialog.cpp
   qgscrashhandler.cpp
 )
 
@@ -393,7 +391,6 @@ SET (QGIS_APP_MOC_HDRS
 
   qgssettingstree.h
   qgsvariantdelegate.h
-  qgscrashdialog.h
   )
 
 

src/app/main.cpp

@@ -877,21 +877,6 @@ int main( int argc, char *argv[] )
   myApp.setWindowIcon( QIcon( QgsApplication::appIconPath() ) );
 #endif
 
-#ifdef Q_OS_WIN
-  if ( !QgsApplication::isRunningFromBuildDir() )
-  {
-    QString symbolPath( getenv( "QGIS_PREFIX_PATH" ) );
-    symbolPath = symbolPath + "\\pdb;http://msdl.microsoft.com/download/symbols;http://download.osgeo.org/osgeo4w/symstore";
-    QgsStackTrace::setSymbolPath( symbolPath );
-  }
-  else
-  {
-    QString symbolPath( getenv( "QGIS_PDB_PATH" ) );
-    symbolPath = symbolPath + ";http://msdl.microsoft.com/download/symbols;http://download.osgeo.org/osgeo4w/symstore";
-    QgsStackTrace::setSymbolPath( symbolPath );
-  }
-#endif
-
   // TODO: use QgsSettings
   QSettings *customizationsettings = nullptr;
 

src/app/qgisapp.cpp

@@ -12093,7 +12093,10 @@ void QgisApp::keyPressEvent( QKeyEvent *e )
 #if defined(_MSC_VER) && defined(QGISDEBUG)
   else if ( e->key() == Qt::Key_Backslash && e->modifiers() & Qt::ControlModifier )
   {
-    QgsCrashHandler::handle( 0 );
+    int *i;
+    *i = 10;
+//      *((int*)0 + 1) = 5;
+//    QgsCrashHandler::handle( 0 );
   }
 #endif
   else

src/app/qgscrashhandler.cpp

@@ -13,46 +13,95 @@
  *   (at your option) any later version.                                   *
  *                                                                         *
  ***************************************************************************/
+
+#include <qgslogger.h>
+#include <iostream>
 #include "qgscrashhandler.h"
+#include "qgsapplication.h"
+#include "qgsproject.h"
 
+#include <gdal.h>
+
+#include <QTextStream>
 #include <QProcess>
 #include <QDir>
-
-#include "qgsproject.h"
-#include "qgscrashdialog.h"
-#include "qgscrashreport.h"
-#include "qgsstacktrace.h"
+#include <QStandardPaths>
+#include <QUuid>
 
 #ifdef _MSC_VER
-LONG WINAPI QgsCrashHandler::handle( struct _EXCEPTION_POINTERS *ExceptionInfo )
+LONG WINAPI QgsCrashHandler::handle( LPEXCEPTION_POINTERS exception )
 {
-  QgsStackLines stack = QgsStackTrace::trace( ExceptionInfo );
-  showCrashDialog( stack );
-
-  return EXCEPTION_EXECUTE_HANDLER;
-}
-#endif
+  QgsDebugMsg( "CRASH!!!" );
 
-void QgsCrashHandler::showCrashDialog( const QgsStackLines &stack )
-{
+  DWORD processID = GetCurrentProcessId();
+  DWORD threadID = GetCurrentThreadId();
 
-  QgsCrashDialog dlg( QApplication::activeWindow() );
-  QgsCrashReport report;
-  report.setStackTrace( stack );
-  dlg.setBugReport( report.toHtml() );
-  if ( dlg.exec() )
+  QString symbolPath;
+  if ( !QgsApplication::isRunningFromBuildDir() )
   {
-    restartApplication();
+    symbolPath = getenv( "QGIS_PREFIX_PATH" );
+    symbolPath = symbolPath + "\\pdb;http://msdl.microsoft.com/download/symbols;http://download.osgeo.org/osgeo4w/symstore";
+  }
+  else
+  {
+    QString pdbPath = getenv( "QGIS_PDB_PATH" );
+    QString appPath = QgsApplication::applicationDirPath();
+    symbolPath += QString( "%1;%2;http://msdl.microsoft.com/download/symbols;http://download.osgeo.org/osgeo4w/symstore" )
+                  .arg( appPath )
+                  .arg( pdbPath );
   }
-}
 
-void QgsCrashHandler::restartApplication()
-{
+  QString ptrStr = QString( "0x%1" ).arg( ( quintptr )exception,
+                                          QT_POINTER_SIZE * 2, 16, QChar( '0' ) );
+  QString fileName = QStandardPaths::standardLocations( QStandardPaths::TempLocation ).at( 0 ) + "/qgis-crash-info-" + QString::number( processID );
+  QgsDebugMsg( fileName );
+
   QStringList arguments;
   arguments = QCoreApplication::arguments();
-  QString path = arguments.at( 0 );
-  arguments.removeFirst();
+  // TODO In future this needs to be moved out into a "session state" file because we can't trust this is valid in
+  // a crash.
   arguments << QgsProject::instance()->fileName();
-  QProcess::startDetached( path, arguments, QDir::toNativeSeparators( QCoreApplication::applicationDirPath() ) );
 
+  QStringList reportData;
+  reportData.append( QStringLiteral( "QGIS Version: %1" ).arg( Qgis::QGIS_VERSION ) );
+
+  if ( QString( Qgis::QGIS_DEV_VERSION ) == QLatin1String( "exported" ) )
+  {
+    reportData.append( QStringLiteral( "QGIS code branch: Release %1.%2" )
+                       .arg( Qgis::QGIS_VERSION_INT / 10000 ).arg( Qgis::QGIS_VERSION_INT / 100 % 100 ) );
+  }
+  else
+  {
+    reportData.append( QStringLiteral( "QGIS code revision: %1" ).arg( Qgis::QGIS_DEV_VERSION ) );
+  }
+
+  reportData.append( QStringLiteral( "Compiled against Qt: %1" ).arg( QT_VERSION_STR ) );
+  reportData.append( QStringLiteral( "Running against Qt: %1" ).arg( qVersion() ) );
+
+  reportData.append( QStringLiteral( "Compiled against GDAL: %1" ).arg( GDAL_RELEASE_NAME ) );
+  reportData.append( QStringLiteral( "Running against GDAL: %1" ).arg( GDALVersionInfo( "RELEASE_NAME" ) ) );
+
+  QFile file( fileName );
+  if ( file.open( QIODevice::WriteOnly | QIODevice::Text ) )
+  {
+    QTextStream stream( &file );
+    stream << QString::number( processID ) << endl;
+    stream << QString::number( threadID ) << endl;
+    stream << ptrStr << endl;
+    stream << '"' + symbolPath + '"' << endl;
+    stream << arguments.join( " " ) << endl;
+    stream << reportData.join( "\n" ) << endl;
+  }
+
+  file.close();
+  QStringList args;
+  args << fileName;
+
+  QString prefixPath( getenv( "QGIS_PREFIX_PATH" ) ? getenv( "QGIS_PREFIX_PATH" ) : QApplication::applicationDirPath() );
+  QString path = prefixPath + "/qgiscrashhandler.exe";
+  QgsDebugMsg( path );
+  QProcess::execute( path, args );
+
+  return TRUE;
 }
+#endif

src/app/qgscrashhandler.h

@@ -18,7 +18,6 @@
 
 #include "qgis.h"
 #include "qgis_app.h"
-#include "qgscrashreport.h"
 
 #ifdef WIN32
 #include <windows.h>
@@ -32,29 +31,15 @@ class APP_EXPORT QgsCrashHandler
 {
 
   public:
-#ifdef _MSC_VER
-    static LONG WINAPI handle( struct _EXCEPTION_POINTERS *ExceptionInfo );
-#endif
-
-    /**
-     * Show the crash dialog.
-     * @param stack The current stack of the crash point.
-     */
-    static void showCrashDialog( const QgsStackLines &stack );
-
-    /**
-     * Restart the application.
-     * Restores project and arguments used when application was loaded.
-     */
-    static void restartApplication();
-
-  private:
-
     /**
      * This class doesn't need to be created by anyone as is only used to handle
      * crashes in the application.
      */
-    QgsCrashHandler() {}
+    QgsCrashHandler() = delete;
+
+#ifdef _MSC_VER
+    static LONG WINAPI handle( LPEXCEPTION_POINTERS ExceptionInfo );
+#endif
 };
 
 

src/core/CMakeLists.txt

@@ -313,7 +313,6 @@ SET(QGIS_CORE_SRCS
   qgsmapthemecollection.cpp
   qgsxmlutils.cpp
   qgssettings.cpp
-  qgsstacktrace.cpp
   qgsarchive.cpp
   qgsziputils.cpp
 
@@ -566,10 +565,6 @@ ELSE(NOT MSVC)
     pal/feature.cpp
     pal/pointset.cpp
     PROPERTIES COMPILE_FLAGS -wd4702)
-  # -wd4091 Avoid 'typedef' ignored on left of '' when no variable is declared warning in DbgHelp.h
-  SET_SOURCE_FILES_PROPERTIES(
-    qgsstacktrace.cpp
-    PROPERTIES COMPILE_FLAGS -wd4091)
 ENDIF(NOT MSVC)
 
 SET(QGIS_CORE_MOC_HDRS

src/crashhandler/CMakeLists.txt

@@ -0,0 +1,32 @@
+INCLUDE_DIRECTORIES(SYSTEM
+  ${CMAKE_CURRENT_BINARY_DIR}
+)
+
+QT5_WRAP_UI(CRASH_UIS_H qgscrashdialog.ui)
+QT5_WRAP_CPP(CRASH_HDR_MOC qgscrashdialog.h)
+
+SET(IMAGE_RCCS ../../images/images.qrc)
+QT5_ADD_RESOURCES(IMAGE_RCC_SRCS ${IMAGE_RCCS})
+
+# -wd4091 Avoid 'typedef' ignored on left of '' when no variable is declared warning in DbgHelp.h
+SET_SOURCE_FILES_PROPERTIES(qgsstacktrace.cpp PROPERTIES COMPILE_FLAGS -wd4091)
+
+ADD_EXECUTABLE(qgiscrashhandler WIN32
+    main.cpp
+    ${CRASH_UIS_H} 
+    ${CRASH_HDR_MOC}
+    ${IMAGE_RCC_SRCS}
+    qgscrashdialog.cpp
+    qgsstacktrace.cpp
+    qgscrashreport.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../app/qgis_win32.rc
+    )
+
+TARGET_LINK_LIBRARIES(qgiscrashhandler
+  ${QT_QTCORE_LIBRARY}
+  ${QT_QTGUI_LIBRARY}
+  DbgHelp
+)
+
+INSTALL(CODE "MESSAGE(\"Installing crashhandler ...\")")
+INSTALL(TARGETS qgiscrashhandler RUNTIME DESTINATION ${QGIS_LIBEXEC_DIR})

src/crashhandler/main.cpp

@@ -0,0 +1,99 @@
+/***************************************************************************
+                            crssync.cpp
+                            -------------------
+   begin                : October 2017
+   copyright            : (C) 2017 by Nathan Woodrow
+   email                : woodrow.nathan@gmail.com
+***************************************************************************/
+
+/***************************************************************************
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ ***************************************************************************/
+#include <memory>
+#include <iostream>
+
+#define _NO_CVCONST_H
+#define _CRT_STDIO_ISO_WIDE_SPECIFIERS
+
+#include <QApplication>
+#include <QMainWindow>
+#include "qgscrashdialog.h"
+#include "qgsstacktrace.h"
+#include "qgscrashreport.h"
+
+
+int main( int argc, char *argv[] )
+{
+  if ( argc < 2 )
+  {
+    std::cout << "QGIS Crash Handler Usage: \n"
+              << "qgscrashhandler {infofile}" << std::endl;
+    return -1;
+  }
+
+  QApplication app( argc, argv );
+  app.setQuitOnLastWindowClosed( true );
+  QCoreApplication::setOrganizationName( "QGIS" );
+  QCoreApplication::setApplicationName( "QGIS3" );
+
+  QString extraInfoFile = QString( argv[1] );
+  std::cout << "Extra Info File: " << extraInfoFile.toUtf8().data() << std::endl;
+
+  QFile file( extraInfoFile );
+  QString processIdString;
+  QString threadIdString;
+  QString exceptionPointersString;
+  QString symbolPaths;
+  QString reloadArgs;
+  QStringList versionInfo;
+
+  if ( file.open( QIODevice::ReadOnly | QIODevice::Text ) )
+  {
+    processIdString = file.readLine();
+    threadIdString = file.readLine();
+    exceptionPointersString = file.readLine();
+    symbolPaths = file.readLine();
+    reloadArgs = file.readLine();
+    // The version info is the last stuff to be in the file until the end
+    // bit gross but :)
+    QString info = file.readAll();
+    versionInfo = info.split( "\n" );
+  }
+
+  DWORD processId;
+  DWORD threadId;
+  LPEXCEPTION_POINTERS exception;
+  processId = processIdString.toULong();
+  threadId = threadIdString.toULong();
+  sscanf_s( exceptionPointersString.toLocal8Bit().constData(), "%p", &exception );
+
+  std::cout << "Process ID: " << processIdString.toLocal8Bit().constData() << std::endl;
+  std::cout << "Trhead ID:" << threadIdString.toLocal8Bit().constData() << std::endl;
+  std::cout << "Exception Pointer: " << exceptionPointersString.toLocal8Bit().constData() << std::endl;
+  std::cout << "Symbol Path :" << symbolPaths.toUtf8().data() << std::endl;
+
+  std::unique_ptr<QgsStackTrace> stackTrace( QgsStackTrace::trace( processId, threadId, exception, symbolPaths ) );
+
+  QgsCrashReport report;
+  report.setVersionInfo( versionInfo );
+  report.setStackTrace( stackTrace.get() );
+  report.exportToCrashFolder();
+
+  QgsCrashDialog dlg;
+  dlg.setReloadArgs( reloadArgs );
+  dlg.setBugReport( report.toHtml() );
+  dlg.setModal( true );
+  dlg.show();
+  app.exec();
+
+  ResumeThread( stackTrace->thread );
+  CloseHandle( stackTrace->thread );
+  CloseHandle( stackTrace->process );
+
+  return 0;
+}