Merge pull request #13007 from Atlante45/fix/crashes-hotfix

Keep CrashpadClient around for application lifetime

interface/src/Crashpad.cpp

@@ -11,6 +11,8 @@
 
 #include "Crashpad.h"
 
+#include <assert.h>
+
 #include <QDebug>
 
 #if HAS_CRASHPAD
@@ -20,7 +22,7 @@
 #include <QStandardPaths>
 #include <QDir>
 
-#include <BuildInfo.h>
+#include <Windows.h>
 
 #include <client/crashpad_client.h>
 #include <client/crash_report_database.h>
@@ -28,28 +30,27 @@
 #include <client/annotation_list.h>
 #include <client/crashpad_info.h>
 
+#include <BuildInfo.h>
+
 using namespace crashpad;
 
 static const std::string BACKTRACE_URL { CMAKE_BACKTRACE_URL };
 static const std::string BACKTRACE_TOKEN { CMAKE_BACKTRACE_TOKEN };
 
-static std::wstring gIPCPipe;
-
 extern QString qAppFileName();
 
+CrashpadClient* client { nullptr };
 std::mutex annotationMutex;
 crashpad::SimpleStringDictionary* crashpadAnnotations { nullptr };
 
-#include <Windows.h>
-
 LONG WINAPI vectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) {
+    if (!client) {
+        return EXCEPTION_CONTINUE_SEARCH;
+    }
+
     if (pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_HEAP_CORRUPTION ||
         pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_STACK_BUFFER_OVERRUN) {
-        CrashpadClient client;
-        if (gIPCPipe.length()) {
-            client.SetHandlerIPCPipe(gIPCPipe);
-        }
-        client.DumpAndCrash(pExceptionInfo);
+        client->DumpAndCrash(pExceptionInfo);
     }
 
     return EXCEPTION_CONTINUE_SEARCH;
@@ -60,7 +61,8 @@ bool startCrashHandler() {
         return false;
     }
 
-    CrashpadClient client;
+    assert(!client);
+    client = new CrashpadClient();
     std::vector<std::string> arguments;
 
     std::map<std::string, std::string> annotations;
@@ -96,12 +98,9 @@ bool startCrashHandler() {
     // Enable automated uploads.
     database->GetSettings()->SetUploadsEnabled(true);
 
-    bool result = client.StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true);
-    gIPCPipe = client.GetHandlerIPCPipe();
-
     AddVectoredExceptionHandler(0, vectoredExceptionHandler);
 
-    return result;
+    return client->StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true);
 }
 
 void setCrashAnnotation(std::string name, std::string value) {

libraries/shared/src/CrashHelpers.cpp

@@ -0,0 +1,77 @@
+//
+//  CrashHelpers.cpp
+//  libraries/shared/src
+//
+//  Created by Clement Brisset on 4/30/18.
+//  Copyright 2018 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#include "CrashHelpers.h"
+
+namespace crash {
+
+class B;
+class A {
+public:
+    A(B* b) : _b(b) { }
+    ~A();
+    virtual void virtualFunction() = 0;
+
+private:
+    B* _b;
+};
+
+class B : public A {
+public:
+    B() : A(this) { }
+    virtual void virtualFunction() override { }
+};
+
+A::~A() {
+    _b->virtualFunction();
+}
+
+void pureVirtualCall() {
+    qCDebug(shared) << "About to make a pure virtual call";
+    B b;
+}
+
+void doubleFree() {
+    qCDebug(shared) << "About to double delete memory";
+    int* blah = new int(200);
+    delete blah;
+    delete blah;
+}
+
+void nullDeref() {
+    qCDebug(shared) << "About to dereference a null pointer";
+    int* p = nullptr;
+    *p = 1;
+}
+
+void doAbort() {
+    qCDebug(shared) << "About to abort";
+    abort();
+}
+
+void outOfBoundsVectorCrash() {
+    qCDebug(shared) << "std::vector out of bounds crash!";
+    std::vector<int> v;
+    v[0] = 42;
+}
+
+void newFault() {
+    qCDebug(shared) << "About to crash inside new fault";
+
+    // Force crash with multiple large allocations
+    while (true) {
+        const size_t GIGABYTE = 1024 * 1024 * 1024;
+        new char[GIGABYTE];
+    }
+
+}
+    
+}

libraries/shared/src/CrashHelpers.h

@@ -18,66 +18,12 @@
 
 namespace crash {
 
-class B;
-class A {
-public:
-    A(B* b) : _b(b) { }
-    ~A();
-    virtual void virtualFunction() = 0;
-
-private:
-    B* _b;
-};
-
-class B : public A {
-public:
-    B() : A(this) { }
-    virtual void virtualFunction() override { }
-};
-
-A::~A() {
-    _b->virtualFunction();
-}
-
-void pureVirtualCall() {
-    qCDebug(shared) << "About to make a pure virtual call";
-    B b;
-}
-    
-void doubleFree() {
-    qCDebug(shared) << "About to double delete memory";
-    int* blah = new int(200);
-    delete blah;
-    delete blah;
-}
-
-void nullDeref() {
-    qCDebug(shared) << "About to dereference a null pointer";
-    int* p = nullptr;
-    *p = 1;
-}
-
-void doAbort() {
-    qCDebug(shared) << "About to abort";
-    abort();
-}
-
-void outOfBoundsVectorCrash() {
-    qCDebug(shared) << "std::vector out of bounds crash!";
-    std::vector<int> v;
-    v[0] = 42;
-}
-
-void newFault() {
-    qCDebug(shared) << "About to crash inside new fault";
-
-    // Force crash with multiple large allocations
-    while (true) {
-        const size_t GIGABYTE = 1024 * 1024 * 1024;
-        new char[GIGABYTE];
-    }
-
-}
+void pureVirtualCall();
+void doubleFree();
+void nullDeref();
+void doAbort();
+void outOfBoundsVectorCrash();
+void newFault();
 
 }