Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows error reporting #524

Merged
merged 4 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Changelog

## [Unreleased]
### Added
- Dialog is shown when fatal error occurs on Windows

### Fixed
- Performance regression on Windows (and potential performance improvements on other platforms)

Expand Down
8 changes: 2 additions & 6 deletions CMakePresets.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"inherits": "base",
"hidden": true,
"cacheVariables": {
"TREMOTESF_QT6": "ON",
"TREMOTESF_WITH_HTTPLIB": "system",
"VCPKG_INSTALLED_DIR": "${sourceDir}/vcpkg-installed",
"VCPKG_INSTALL_OPTIONS": "--disable-metrics;--clean-buildtrees-after-build;--clean-packages-after-build",
Expand All @@ -43,11 +44,9 @@
"strategy": "external"
},
"cacheVariables": {
"TREMOTESF_QT6": "ON",
"VCPKG_TARGET_TRIPLET": "x64-windows-static",
"VCPKG_HOST_TRIPLET": "x64-windows-static"
},
"toolchainFile": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake",
"vendor": {
"microsoft.com/VisualStudioSettings/CMake/1.0": {
"hostOS": [
Expand All @@ -73,9 +72,6 @@
"type": "equals",
"lhs": "${hostSystemName}",
"rhs": "Darwin"
},
"cacheVariables": {
"TREMOTESF_QT6": "ON"
}
},
{
Expand Down Expand Up @@ -301,4 +297,4 @@
"configuration": "RelWithDebInfo"
}
]
}
}
12 changes: 10 additions & 2 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ target_compile_definitions(
TREMOTESF_APP_ID="org.equeim.Tremotesf"
TREMOTESF_APP_NAME="${TREMOTESF_APP_NAME}"
TREMOTESF_VERSION="${PROJECT_VERSION}"
FMT_VERSION_MAJOR=${fmt_VERSION_MAJOR}
)
if (registrable_domain_qt)
target_compile_definitions(tremotesf_objects PUBLIC TREMOTESF_REGISTRABLE_DOMAIN_QT)
Expand Down Expand Up @@ -314,8 +315,8 @@ elseif (WIN32)
ipc/ipcserver_socket.cpp
ipc/ipcserver_socket.h
startup/signalhandler_windows.cpp
startup/windowsmessagehandler.cpp
startup/windowsmessagehandler.h
startup/windowsfatalerrorhandlers.cpp
startup/windowsfatalerrorhandlers.h
ui/darkthemeapplier_windows.h
ui/darkthemeapplier_windows.cpp
ui/notificationscontroller_fallback.cpp
Expand All @@ -339,13 +340,20 @@ if (WIN32)
PRIVATE
startup/main_windows.cpp
startup/main_windows.h
startup/windowsmessagehandler.cpp
startup/windowsmessagehandler.h
"${CMAKE_CURRENT_BINARY_DIR}/tremotesf.rc"
)
if (MSVC)
target_sources(tremotesf PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/tremotesf.manifest")
else ()
target_sources(tremotesf PRIVATE tremotesf.manifest.rc)
endif ()
if (MSVC)
# For C++23's std::stacktrace
set_source_files_properties(startup/windowsfatalerrorhandlers.cpp PROPERTIES COMPILE_OPTIONS "/std:c++latest")
target_link_options(tremotesf PRIVATE "/PDBALTPATH:%_PDB%")
endif()
set_target_properties(tremotesf PROPERTIES WIN32_EXECUTABLE $<NOT:$<CONFIG:Debug>>)
endif ()

Expand Down
4 changes: 2 additions & 2 deletions src/coroutines/coroutines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ namespace tremotesf::impl {
}

void CoroutinePromiseBase::abortNoParent(std::coroutine_handle<> handle) {
warning().log("No parent coroutine when completing coroutine {}", handle.address());
std::abort();
fatal().log("No parent coroutine when completing coroutine {}", handle.address());
Q_UNREACHABLE();
}

std::coroutine_handle<> CoroutinePromise<void>::onPerformedFinalSuspend() {
Expand Down
8 changes: 4 additions & 4 deletions src/coroutines/coroutines.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,10 +277,10 @@ namespace tremotesf {
}
}

#define cancelCoroutine() \
do { \
co_await tremotesf::impl::CancellationAwaiter{}; \
std::abort(); /* CancellationAwaiter must never resume */ \
#define cancelCoroutine() \
do { \
co_await tremotesf::impl::CancellationAwaiter{}; \
qFatal("CancellationAwaiter resumed"); /* CancellationAwaiter must never resume */ \
} while (false);

#endif // TREMOTESF_COROUTINES_H
4 changes: 2 additions & 2 deletions src/coroutines/scope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ namespace tremotesf {
handleException(unhandledException);
const auto found = std::ranges::find(mCoroutines, coroutine);
if (found == mCoroutines.end()) {
warning().log("Did not find completed coroutine {} in CoroutineScope", coroutine->address());
std::abort();
fatal().log("Did not find completed coroutine {} in CoroutineScope", coroutine->address());
Q_UNREACHABLE();
}
mCoroutines.erase(found);
delete coroutine;
Expand Down
7 changes: 7 additions & 0 deletions src/coroutines/threadpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@

#include "coroutines.h"

#ifdef Q_OS_WIN
# include "startup/windowsfatalerrorhandlers.h"
#endif

namespace tremotesf {
namespace impl {
template<std::invocable Function, typename T = std::invoke_result_t<Function>>
Expand Down Expand Up @@ -67,6 +71,9 @@ namespace tremotesf {
if (mSharedData->cancelled) {
return;
}
#ifdef Q_OS_WIN
windowsSetUpFatalErrorHandlersInThread();
#endif
try {
mSharedData->result = mFunction();
} catch (...) {
Expand Down
4 changes: 2 additions & 2 deletions src/coroutines/waitall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ namespace tremotesf::impl {
}
const auto found = std::ranges::find_if(mCoroutines, [coroutine](auto& c) { return &c == coroutine; });
if (found == mCoroutines.end()) {
warning().log("Did not find completed coroutine {} in MultipleCoroutinesAwaiter", coroutine->address());
std::abort();
fatal().log("Did not find completed coroutine {} in MultipleCoroutinesAwaiter", coroutine->address());
Q_UNREACHABLE();
}
mCoroutines.erase(found);
if (mCancellingCoroutines) {
Expand Down
2 changes: 1 addition & 1 deletion src/log/formatters.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
# include <QUtf8StringView>
#endif

#if __has_include(<fmt/base.h>)
#if FMT_VERSION_MAJOR >= 11
# include <fmt/base.h>
#else
# include <fmt/core.h>
Expand Down
72 changes: 47 additions & 25 deletions src/log/log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,54 @@ namespace tremotesf {
qputenv(loggingRulesEnvVariable, rules);
}

namespace {
template<typename T>
void appendNestedException(std::string& out, const T& e) {
fmt::format_to(std::back_insert_iterator(out), "\nCaused by: {}", e);
}

template<typename T>
void appendNestedExceptions(std::string& out, const T& e) {
try {
std::rethrow_if_nested(e);
} catch (const std::system_error& nested) {
appendNestedException(out, nested);
appendNestedExceptions(out, nested);
} catch (const std::exception& nested) {
appendNestedException(out, nested);
appendNestedExceptions(out, nested);
}
#ifdef Q_OS_WIN
catch (const winrt::hresult_error& nested) {
appendNestedException(out, nested);
appendNestedExceptions(out, nested);
}
#endif
catch (...) {
fmt::format_to(std::back_insert_iterator(out), "\nCaused by: unknown exception");
}
}
}

template<impl::IsException E>
std::string formatExceptionRecursivelyImpl(const E& e) {
std::string out = fmt::format(impl::singleArgumentFormatString, e);
appendNestedExceptions(out, e);
return out;
}

std::string formatExceptionRecursively(const std::exception& e) { return formatExceptionRecursivelyImpl(e); }
std::string formatExceptionRecursively(const std::system_error& e) { return formatExceptionRecursivelyImpl(e); }
#ifdef Q_OS_WIN
std::string formatExceptionRecursively(const winrt::hresult_error& e) { return formatExceptionRecursivelyImpl(e); }
#endif

QString Logger::formatToQString(fmt::string_view fmt, fmt::format_args args) {
return QString::fromStdString(fmt::vformat(fmt, args));
}

void Logger::logWithFormatArgs(fmt::string_view fmt, fmt::format_args args) const {
logImpl(QString::fromStdString(fmt::vformat(fmt, args)));
logImpl(formatToQString(fmt, args));
}

void Logger::logImpl(const QString& string) const {
Expand All @@ -36,28 +82,4 @@ namespace tremotesf {
// 2. QMessageLogger::<>(const char*, ...) overloads perform QString::vasprintf() formatting
qt_message_output(type, context, string);
}

template<impl::IsException E>
void Logger::logExceptionRecursively(const E& e) const {
logImpl(QString::fromStdString(fmt::format(" |- Caused by: {}", e)));
try {
std::rethrow_if_nested(e);
} catch (const std::exception& nested) {
logExceptionRecursively(nested);
}
#ifdef Q_OS_WIN
catch (const winrt::hresult_error& nested) {
logExceptionRecursively(nested);
}
#endif
catch (...) {
logImpl(QStringLiteral(" |- Caused by: unknown exception"));
}
}

template void Logger::logExceptionRecursively<std::exception>(const std::exception&) const;
template void Logger::logExceptionRecursively<std::system_error>(const std::system_error&) const;
#ifdef Q_OS_WIN
template void Logger::logExceptionRecursively<winrt::hresult_error>(const winrt::hresult_error&) const;
#endif
}
54 changes: 24 additions & 30 deletions src/log/log.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@

#include <concepts>
#include <type_traits>
#include <string>
#include <source_location>

#include <QLoggingCategory>
#include <QMessageLogger>
#include <QString>

#if __has_include(<fmt/base.h>)
#if FMT_VERSION_MAJOR >= 11
# include <fmt/base.h>
#else
# include <fmt/core.h>
Expand Down Expand Up @@ -82,6 +83,12 @@ namespace tremotesf {

void overrideDebugLogs(bool enable);

std::string formatExceptionRecursively(const std::exception& e);
std::string formatExceptionRecursively(const std::system_error& e);
#ifdef Q_OS_WIN
std::string formatExceptionRecursively(const winrt::hresult_error& e);
#endif

struct Logger {
ALWAYS_INLINE consteval explicit Logger(QtMsgType type, std::source_location location)
: type(type),
Expand Down Expand Up @@ -134,61 +141,44 @@ namespace tremotesf {
template<impl::IsException E, typename T>
ALWAYS_INLINE void logWithException(const E& e, const T& value) const {
if (isEnabled()) {
const auto formattedException = formatExceptionRecursively(e);
logWithFormatArgs(
fmt::format_string<const T&>(impl::singleArgumentFormatString),
fmt::make_format_args(value)
fmt::format_string<const T&, const std::string&>("{}\n{}"),
fmt::make_format_args(value, formattedException)
);
logExceptionRecursivelyWrapper(e);
}
}

template<impl::IsException E, typename... Args>
requires(sizeof...(Args) != 0)
ALWAYS_INLINE void logWithException(const E& e, fmt::format_string<Args...> fmt, const Args&... args) const {
if (isEnabled()) {
logWithFormatArgs(fmt, fmt::make_format_args(args...));
logExceptionRecursivelyWrapper(e);
auto message = formatToQString(fmt, fmt::make_format_args(args...));
message += '\n';
#if QT_VERSION >= QT_VERSION_CHECK(6, 5, 0)
message += formatExceptionRecursively(e);
#else
message += formatExceptionRecursively(e).c_str();
#endif
logImpl(message);
}
}

private:
ALWAYS_INLINE bool isEnabled() const { return tremotesfLoggingCategory().isEnabled(type); }

static QString formatToQString(fmt::string_view fmt, fmt::format_args args);
void logWithFormatArgs(fmt::string_view fmt, fmt::format_args args) const;

/**
* Actual log function
*/
void logImpl(const QString& string) const;

template<impl::IsException E>
ALWAYS_INLINE void logExceptionRecursivelyWrapper(const E& e) const {
if constexpr (std::derived_from<std::remove_cvref_t<E>, std::system_error>) {
logExceptionRecursively(static_cast<const std::system_error&>(e));
}
#ifdef Q_OS_WIN
else if constexpr (std::derived_from<std::remove_cvref_t<E>, winrt::hresult_error>) {
logExceptionRecursively(static_cast<const winrt::hresult_error&>(e));
}
#endif
else {
logExceptionRecursively(static_cast<const std::exception&>(e));
}
}

template<impl::IsException E>
void logExceptionRecursively(const E& e) const;

QtMsgType type;
QMessageLogContext context;
};

extern template void Logger::logExceptionRecursively<std::exception>(const std::exception&) const;
extern template void Logger::logExceptionRecursively<std::system_error>(const std::system_error&) const;
#ifdef Q_OS_WIN
extern template void Logger::logExceptionRecursively<winrt::hresult_error>(const winrt::hresult_error&) const;
#endif

ALWAYS_INLINE consteval Logger debug(std::source_location location = std::source_location::current()) {
return Logger(QtDebugMsg, location);
}
Expand All @@ -201,6 +191,10 @@ namespace tremotesf {
return Logger(QtWarningMsg, location);
}

ALWAYS_INLINE consteval Logger fatal(std::source_location location = std::source_location::current()) {
return Logger(QtFatalMsg, location);
}

template<typename T>
ALWAYS_INLINE void printlnStdout(const T& value) {
fmt::print(stdout, fmt::format_string<const T&>(impl::singleArgumentFormatString), value);
Expand Down
Loading
Loading