Skip to content

Loading…

Windows: Create minidumps for threads terminated by exceptions #813

Closed
wants to merge 3 commits into from

6 participants

@CrystalP

Hopefully with this and build symbol files it will be easier to figure out what crashed XBMC or a thread on a user's computer.

The PR will create the dumps in the same directory as the debug logs, with file names like:
XBMC majorminortag gitrev datetime.dmp

In case the exception happens before XBMC directories settings were initialized (unlikely), I'm not sure what's the best location for the dump. I thought of a few possibilities:

  • in the same directory as the exe. Maybe a problem for restricted users? It's what the PR implements at the moment
  • in the root of a user's Windows profile
  • in Windows temp directory. Same as the task manager would do.

What do you think, keeping in mind there are regular and portable installs?

@jmarshallnz
Team Kodi member

Looks fine to me. For exceptions at startup I'd dump it in temp I think as at least you know it's writeable.

@CrystalP

I also favor temp.
But I'll have to redo this PR as I found there already was minidump creation code in XBMC_PC.cpp and it seems silly to duplicate it.

@wsoltys
Team Kodi member

The current mini dump code is only executed if XBMC got a uncaught exception and exits. You might wanna change that to support the rest of the caught exceptions. The dumps shows not much without the pdbs which you already requested from TheUni. I think for 80% of the current exceptions the xbmc.pdb is enough :)

@Montellese
Team Kodi member

Nice work. This should make tracking down those win32 crashes quite a bit easier. I'd favor the windows temp directory as well as it should always be writeable.

Is there already a way for us to get the PDB of billy's builds?

@CrystalP

Here it is reworked, dumping on exception in any thread. I need to test a bit more, but that's pretty much it.
Handling on main thread is a bit inconsistent, to be fixed later.
The pdb files are supposed to be automatically archived now. We'll see when nightlies are turned on.

@Montellese
Team Kodi member

The PDB of the test build TheUni did yesterday was automatically uploaded alongside the EXE, see http://mirrors.xbmc.org/test-builds/win32/

@theuni
Team Kodi member

that one was done manually. from now on, they'll be alongside nightly builds. last night's was the first to run successfully: http://mirrors.xbmc.org/nightlies/win32/

@CrystalP

OK, works fine most of the time. The dmp file is not always valid though and dumping with more than minimal information sometimes hangs - maybe that has to do with the recommendation to do the dump from another process.

It's already better than nothing though, so I think it should go in the May merge window.

@wsoltys
Team Kodi member

pull it in.

@dersphere
Team Kodi member

Should I include these .dmp files to be uploaded by the "XBMC Log Uploader"-Add-on? Crashlogs for OSX/IOS/Linux are already included.

@CrystalP

That'd be a good thing, however how would you know which file(s) to upload?
The .dmp files are going to pile up from one XBMC session to the next. Even the same session could generate multiple .dmp
Maybe I should add a cleanup to avoid accumulating too many dumps, or at the start of xbmc delete all previous .dmp files.
That could be added later, I don't consider it blocking for merge.

@dersphere
Team Kodi member

At the moment the add-on looks for matching files (would be .dmp in that case) and asks the user if he want the most recent (sorted by mtime) to be uploaded.

@CrystalP

That would work most often.

@CrystalP CrystalP was assigned
@wsoltys
Team Kodi member

rebased #1285

@wsoltys wsoltys closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 103 additions and 75 deletions.
  1. +5 −0 xbmc/threads/Thread.cpp
  2. +86 −9 xbmc/utils/Win32Exception.cpp
  3. +8 −2 xbmc/utils/Win32Exception.h
  4. +4 −64 xbmc/win32/XBMC_PC.cpp
View
5 xbmc/threads/Thread.cpp
@@ -161,6 +161,7 @@ DWORD WINAPI CThread::staticThread(LPVOID* data)
catch (const win32_exception &e)
{
e.writelog(__FUNCTION__);
+ e.write_minidump();
if( pThread->IsAutoDelete() )
{
delete pThread;
@@ -190,10 +191,12 @@ DWORD WINAPI CThread::staticThread(LPVOID* data)
catch (const access_violation &e)
{
e.writelog(__FUNCTION__);
+ e.write_minidump();
}
catch (const win32_exception &e)
{
e.writelog(__FUNCTION__);
+ e.write_minidump();
}
#endif
catch(...)
@@ -209,10 +212,12 @@ DWORD WINAPI CThread::staticThread(LPVOID* data)
catch (const access_violation &e)
{
e.writelog(__FUNCTION__);
+ e.write_minidump();
}
catch (const win32_exception &e)
{
e.writelog(__FUNCTION__);
+ e.write_minidump();
}
#endif
catch(...)
View
95 xbmc/utils/Win32Exception.cpp
@@ -22,6 +22,9 @@
#include "Win32Exception.h"
#ifndef _LINUX
#include "eh.h"
+#include <dbghelp.h>
+#include "Util.h"
+#include "WIN32Util.h"
#endif
#include "log.h"
@@ -40,6 +43,13 @@ void win32_exception::writelog(const char *prefix) const
#else
+typedef BOOL (WINAPI *MINIDUMPWRITEDUMP)(HANDLE hProcess, DWORD dwPid, HANDLE hFile, MINIDUMP_TYPE DumpType,
+ CONST PMINIDUMP_EXCEPTION_INFORMATION ExceptionParam,
+ CONST PMINIDUMP_USER_STREAM_INFORMATION UserStreamParam,
+ CONST PMINIDUMP_CALLBACK_INFORMATION CallbackParam);
+
+CStdString win32_exception::mVersion;
+
void win32_exception::install_handler()
{
_set_se_translator(win32_exception::translate);
@@ -47,20 +57,20 @@ void win32_exception::install_handler()
void win32_exception::translate(unsigned code, EXCEPTION_POINTERS* info)
{
- // Windows guarantees that *(info->ExceptionRecord) is valid
switch (code) {
case EXCEPTION_ACCESS_VIOLATION:
- throw access_violation(*(info->ExceptionRecord));
+ throw access_violation(info);
break;
default:
- throw win32_exception(*(info->ExceptionRecord));
+ throw win32_exception(info);
}
}
-win32_exception::win32_exception(const EXCEPTION_RECORD& info)
-: mWhat("Win32 exception"), mWhere(info.ExceptionAddress), mCode(info.ExceptionCode)
+win32_exception::win32_exception(EXCEPTION_POINTERS* info)
+: mWhat("Win32 exception"), mWhere(info->ExceptionRecord->ExceptionAddress), mCode(info->ExceptionRecord->ExceptionCode), mExceptionPointers(info)
{
- switch (info.ExceptionCode) {
+ // Windows guarantees that *(info->ExceptionRecord) is valid
+ switch (info->ExceptionRecord->ExceptionCode) {
case EXCEPTION_ACCESS_VIOLATION:
mWhat = "Access violation";
break;
@@ -79,10 +89,77 @@ void win32_exception::writelog(const char *prefix) const
CLog::Log(LOGERROR, "%s (code:0x%08x) at 0x%08x", what(), code(), where());
}
-access_violation::access_violation(const EXCEPTION_RECORD& info)
+bool win32_exception::write_minidump(EXCEPTION_POINTERS* pEp)
+{
+ // Create the dump file where the xbmc.exe resides
+ bool returncode = false;
+ CStdString dumpFileName;
+ SYSTEMTIME stLocalTime;
+ GetLocalTime(&stLocalTime);
+
+ dumpFileName.Format("xbmc crashlog %s %04d%02d%02d-%02d%02d%02d.dmp",
+ mVersion.c_str(),
+ stLocalTime.wYear, stLocalTime.wMonth, stLocalTime.wDay,
+ stLocalTime.wHour, stLocalTime.wMinute, stLocalTime.wSecond);
+
+ dumpFileName.Format("%s\\%s", CWIN32Util::GetProfilePath().c_str(), CUtil::MakeLegalFileName(dumpFileName));
+
+ HANDLE hDumpFile = CreateFile(dumpFileName.c_str(), GENERIC_WRITE, 0, 0, CREATE_ALWAYS, 0, 0);
+
+ if (hDumpFile == INVALID_HANDLE_VALUE)
+ {
+ CLog::Log(LOGERROR, "CreateFile '%s' failed with error id %d", dumpFileName.c_str(), GetLastError());
+ goto cleanup;
+ }
+
+ // Load the DBGHELP DLL
+ HMODULE hDbgHelpDll = ::LoadLibrary("DBGHELP.DLL");
+ if (!hDbgHelpDll)
+ {
+ CLog::Log(LOGERROR, "LoadLibrary 'DBGHELP.DLL' failed with error id %d", GetLastError());
+ goto cleanup;
+ }
+
+ MINIDUMPWRITEDUMP pDump = (MINIDUMPWRITEDUMP)::GetProcAddress(hDbgHelpDll, "MiniDumpWriteDump");
+ if (!pDump)
+ {
+ CLog::Log(LOGERROR, "Failed to locate MiniDumpWriteDump with error id %d", GetLastError());
+ goto cleanup;
+ }
+
+ // Initialize minidump structure
+ MINIDUMP_EXCEPTION_INFORMATION mdei;
+ mdei.ThreadId = GetCurrentThreadId();
+ mdei.ExceptionPointers = pEp;
+ mdei.ClientPointers = FALSE;
+
+ // Call the minidump api with normal dumping
+ // We can get more detail information by using other minidump types but the dump file will be
+ // extremely large.
+ BOOL bMiniDumpSuccessful = pDump(GetCurrentProcess(), GetCurrentProcessId(), hDumpFile, MiniDumpNormal, &mdei, 0, NULL);
+ if( !bMiniDumpSuccessful )
+ {
+ CLog::Log(LOGERROR, "MiniDumpWriteDump failed with error id %d", GetLastError());
+ goto cleanup;
+ }
+
+ returncode = true;
+
+cleanup:
+
+ if (hDumpFile != INVALID_HANDLE_VALUE)
+ CloseHandle(hDumpFile);
+
+ if (hDbgHelpDll)
+ FreeLibrary(hDbgHelpDll);
+
+ return returncode;
+}
+
+access_violation::access_violation(EXCEPTION_POINTERS* info)
: win32_exception(info), mAccessType(Invalid), mBadAddress(0)
{
- switch(info.ExceptionInformation[0])
+ switch(info->ExceptionRecord->ExceptionInformation[0])
{
case 0:
mAccessType = Read;
@@ -94,7 +171,7 @@ access_violation::access_violation(const EXCEPTION_RECORD& info)
mAccessType = DEP;
break;
}
- mBadAddress = reinterpret_cast<win32_exception ::Address>(info.ExceptionInformation[1]);
+ mBadAddress = reinterpret_cast<win32_exception ::Address>(info->ExceptionRecord->ExceptionInformation[1]);
}
void access_violation::writelog(const char *prefix) const
View
10 xbmc/utils/Win32Exception.h
@@ -25,6 +25,7 @@
#include <windows.h>
#endif
#include <exception>
+#include "utils/StdString.h"
#ifdef _LINUX
@@ -49,17 +50,22 @@ class win32_exception: public std::exception
typedef const void* Address; // OK on Win32 platform
static void install_handler();
+ static void set_version(CStdString version) { mVersion = version; };
virtual const char* what() const { return mWhat; };
Address where() const { return mWhere; };
unsigned code() const { return mCode; };
virtual void writelog(const char *prefix) const;
+ bool write_minidump() const { return write_minidump(mExceptionPointers); };
+ static bool write_minidump(EXCEPTION_POINTERS* pEp);
protected:
- win32_exception(const EXCEPTION_RECORD& info);
+ win32_exception(EXCEPTION_POINTERS* info);
static void translate(unsigned code, EXCEPTION_POINTERS* info);
private:
const char* mWhat;
Address mWhere;
unsigned mCode;
+ EXCEPTION_POINTERS *mExceptionPointers;
+ static CStdString mVersion;
};
class access_violation: public win32_exception
@@ -80,6 +86,6 @@ class access_violation: public win32_exception
private:
access_type mAccessType;
Address mBadAddress;
- access_violation(const EXCEPTION_RECORD& info);
+ access_violation(EXCEPTION_POINTERS* info);
};
#endif
View
68 xbmc/win32/XBMC_PC.cpp
@@ -23,79 +23,18 @@
#include "settings/AppParamParser.h"
#include "utils/CharsetConverter.h"
#include "utils/log.h"
-#include "WIN32Util.h"
+#include "utils/Win32Exception.h"
#include "shellapi.h"
#include "dbghelp.h"
#include "XBDateTime.h"
#include "threads/Thread.h"
#include "Application.h"
-
-typedef BOOL (WINAPI *MINIDUMPWRITEDUMP)(HANDLE hProcess, DWORD dwPid, HANDLE hFile, MINIDUMP_TYPE DumpType,
- CONST PMINIDUMP_EXCEPTION_INFORMATION ExceptionParam,
- CONST PMINIDUMP_USER_STREAM_INFORMATION UserStreamParam,
- CONST PMINIDUMP_CALLBACK_INFORMATION CallbackParam);
-
+#include "GUIInfoManager.h"
// Minidump creation function
LONG WINAPI CreateMiniDump( EXCEPTION_POINTERS* pEp )
{
- // Create the dump file where the xbmc.exe resides
- CStdString errorMsg;
- CStdString dumpFile;
- CDateTime now(CDateTime::GetCurrentDateTime());
- dumpFile.Format("%s\\xbmc_crashlog-%04i%02i%02i-%02i%02i%02i.dmp", CWIN32Util::GetProfilePath().c_str(), now.GetYear(), now.GetMonth(), now.GetDay(), now.GetHour(), now.GetMinute(), now.GetSecond());
- HANDLE hFile = CreateFile(dumpFile.c_str(), GENERIC_READ | GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL );
-
- // Call MiniDumpWriteDump api with the dump file
- if ( hFile && ( hFile != INVALID_HANDLE_VALUE ) )
- {
- // Load the DBGHELP DLL
- HMODULE hDbgHelpDll = ::LoadLibrary("DBGHELP.DLL");
- if (hDbgHelpDll)
- {
- MINIDUMPWRITEDUMP pDump = (MINIDUMPWRITEDUMP)::GetProcAddress(hDbgHelpDll, "MiniDumpWriteDump");
- if (pDump)
- {
- // Initialize minidump structure
- MINIDUMP_EXCEPTION_INFORMATION mdei;
- mdei.ThreadId = CThread::GetCurrentThreadId();
- mdei.ExceptionPointers = pEp;
- mdei.ClientPointers = FALSE;
-
- // Call the minidump api with normal dumping
- // We can get more detail information by using other minidump types but the dump file will be
- // extermely large.
- BOOL rv = pDump(GetCurrentProcess(), GetCurrentProcessId(), hFile, MiniDumpNormal, &mdei, 0, NULL);
- if( !rv )
- {
- errorMsg.Format("MiniDumpWriteDump failed with error id %d", GetLastError());
- MessageBox(NULL, errorMsg.c_str(), "XBMC: Error", MB_OK|MB_ICONERROR);
- }
- }
- else
- {
- errorMsg.Format("MiniDumpWriteDump failed to load with error id %d", GetLastError());
- MessageBox(NULL, errorMsg.c_str(), "XBMC: Error", MB_OK|MB_ICONERROR);
- }
-
- // Close the DLL
- FreeLibrary(hDbgHelpDll);
- }
- else
- {
- errorMsg.Format("LoadLibrary 'DBGHELP.DLL' failed with error id %d", GetLastError());
- MessageBox(NULL, errorMsg.c_str(), "XBMC: Error", MB_OK|MB_ICONERROR);
- }
-
- // Close the file
- CloseHandle( hFile );
- }
- else
- {
- errorMsg.Format("CreateFile '%s' failed with error id %d", dumpFile.c_str(), GetLastError());
- MessageBox(NULL, errorMsg.c_str(), "XBMC: Error", MB_OK|MB_ICONERROR);
- }
-
+ win32_exception::write_minidump(pEp);
return pEp->ExceptionRecord->ExceptionCode;;
}
@@ -117,6 +56,7 @@ INT WINAPI WinMain( HINSTANCE hInst, HINSTANCE, LPSTR commandLine, INT )
CLog::SetLogLevel(g_advancedSettings.m_logLevel);
// Initializes CreateMiniDump to handle exceptions.
+ win32_exception::set_version(g_infoManager.GetVersion());
SetUnhandledExceptionFilter( CreateMiniDump );
// check if XBMC is already running
Something went wrong with that request. Please try again.