From 8a2a9d2809ed0664c2e47d634e1c2c6c6493d251 Mon Sep 17 00:00:00 2001 From: Luboš Luňák Date: Wed, 5 Feb 2020 11:56:40 +0100 Subject: add Skia crash zone checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just like with OpenGL, mark zones where Skia code may call into graphics drivers in order to be able to detect cases where the driver has caused a crash or a lockup. Change-Id: I0fdbcc2260e3ab7821a595e9960145ae1fc1adfe Reviewed-on: https://gerrit.libreoffice.org/c/core/+/88011 Tested-by: Jenkins Reviewed-by: Luboš Luňák --- vcl/Library_vcl.mk | 1 + vcl/inc/skia/zone.hxx | 30 +++++++++++++++++++++++ vcl/skia/SkiaHelper.cxx | 7 ++++++ vcl/skia/gdiimpl.cxx | 12 ++++++++++ vcl/skia/salbmp.cxx | 7 ++++++ vcl/skia/win/gdiimpl.cxx | 7 ++++++ vcl/skia/x11/gdiimpl.cxx | 4 ++++ vcl/skia/zone.cxx | 58 +++++++++++++++++++++++++++++++++++++++++++++ vcl/source/app/svmain.cxx | 5 ++++ vcl/source/app/watchdog.cxx | 7 ++++++ 10 files changed, 138 insertions(+) create mode 100644 vcl/inc/skia/zone.hxx create mode 100644 vcl/skia/zone.cxx diff --git a/vcl/Library_vcl.mk b/vcl/Library_vcl.mk index 16be92968a92..6728447a7817 100644 --- a/vcl/Library_vcl.mk +++ b/vcl/Library_vcl.mk @@ -592,6 +592,7 @@ $(eval $(call gb_Library_add_exception_objects,vcl,\ $(if $(filter SKIA,$(BUILD_TYPE)), \ vcl/skia/packedsurfaceatlas \ vcl/skia/salbmp \ + vcl/skia/zone \ vcl/skia/gdiimpl) \ )) diff --git a/vcl/inc/skia/zone.hxx b/vcl/inc/skia/zone.hxx new file mode 100644 index 000000000000..1f6bbb0dddca --- /dev/null +++ b/vcl/inc/skia/zone.hxx @@ -0,0 +1,30 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#ifndef INCLUDED_VCL_INC_SKIA_ZONE_H +#define INCLUDED_VCL_INC_SKIA_ZONE_H + +#include + +#include + +// Used around calls to Skia code to detect crashes in drivers. +class VCL_DLLPUBLIC SkiaZone : public CrashZone +{ +public: + static void hardDisable(); + static void relaxWatchdogTimings(); + static const CrashWatchdogTimingsValues& getCrashWatchdogTimingsValues(); + static void checkDebug(int nUnchanged, const CrashWatchdogTimingsValues& aTimingValues); + static const char* name() { return "Skia"; } +}; + +#endif // INCLUDED_VCL_INC_SKIA_ZONE_H + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/skia/SkiaHelper.cxx b/vcl/skia/SkiaHelper.cxx index 20bf299cfb56..281fdaf65f77 100644 --- a/vcl/skia/SkiaHelper.cxx +++ b/vcl/skia/SkiaHelper.cxx @@ -12,6 +12,8 @@ #include #include #include +#include +#include #if !HAVE_FEATURE_SKIA @@ -94,6 +96,9 @@ bool isVCLSkiaEnabled() bRet = bEnable; } + if (bRet) + WatchdogThread::start(); + CrashReporter::addKeyValue("UseSkia", OUString::boolean(bRet), CrashReporter::Write); return bRet; @@ -135,6 +140,7 @@ static sk_app::VulkanWindowContext::SharedGrContext* sharedGrContext; GrContext* getSharedGrContext() { + SkiaZone zone; assert(renderMethodToUse() == RenderVulkan); if (sharedGrContext) return sharedGrContext->getGrContext(); @@ -157,6 +163,7 @@ GrContext* getSharedGrContext() sk_sp createSkSurface(int width, int height, SkColorType type) { + SkiaZone zone; assert(type == kN32_SkColorType || type == kAlpha_8_SkColorType); sk_sp surface; switch (SkiaHelper::renderMethodToUse()) diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx index 7ec10165f5d0..a1d8c00674d8 100644 --- a/vcl/skia/gdiimpl.cxx +++ b/vcl/skia/gdiimpl.cxx @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -197,6 +198,7 @@ void SkiaSalGraphicsImpl::recreateSurface() void SkiaSalGraphicsImpl::createSurface() { + SkiaZone zone; if (isOffscreen()) createOffscreenSurface(); else @@ -211,6 +213,7 @@ void SkiaSalGraphicsImpl::createSurface() void SkiaSalGraphicsImpl::createWindowSurface() { + SkiaZone zone; assert(!isOffscreen()); assert(!mSurface); assert(!mWindowContext); @@ -239,6 +242,7 @@ void SkiaSalGraphicsImpl::createWindowSurface() void SkiaSalGraphicsImpl::createOffscreenSurface() { + SkiaZone zone; assert(isOffscreen()); assert(!mSurface); assert(!mWindowContext); @@ -284,6 +288,7 @@ void SkiaSalGraphicsImpl::createOffscreenSurface() void SkiaSalGraphicsImpl::destroySurface() { + SkiaZone zone; if (mSurface) { // check setClipRegion() invariant @@ -309,6 +314,7 @@ void SkiaSalGraphicsImpl::DeInit() { destroySurface(); } void SkiaSalGraphicsImpl::preDraw() { + SkiaZone::enter(); // matched in postDraw() checkSurface(); assert(!mXorMode || mXorExtents.isEmpty()); // must be reset in postDraw() } @@ -390,6 +396,7 @@ void SkiaSalGraphicsImpl::postDraw() mSurface->flush(); mPendingPixelsToFlush = 0; } + SkiaZone::leave(); // matched in preDraw() } // VCL can sometimes resize us without telling us, update the surface if needed. @@ -420,6 +427,7 @@ bool SkiaSalGraphicsImpl::setClipRegion(const vcl::Region& region) { if (mClipRegion == region) return true; + SkiaZone zone; mClipRegion = region; checkSurface(); SAL_INFO("vcl.skia", "setclipregion(" << this << "): " << region); @@ -438,6 +446,7 @@ bool SkiaSalGraphicsImpl::setClipRegion(const vcl::Region& region) void SkiaSalGraphicsImpl::setCanvasClipRegion(SkCanvas* canvas, const vcl::Region& region) { + SkiaZone zone; SkPath path; // Handle polygons last, since rectangle->polygon area conversions // are problematic (see addPolygonToPath() comment). @@ -485,6 +494,7 @@ void SkiaSalGraphicsImpl::SetXORMode(bool set, bool) SkCanvas* SkiaSalGraphicsImpl::getXorCanvas() { + SkiaZone zone; assert(mXorMode); // Skia does not implement xor drawing, so we need to handle it manually by redirecting // to a temporary SkBitmap and then doing the xor operation on the data ourselves. @@ -960,6 +970,7 @@ void SkiaSalGraphicsImpl::drawMask(const SalTwoRect& rPosAry, const sk_sp SkiaSalGraphicsImpl::getBitmap(long nX, long nY, long nWidth, long nHeight) { + SkiaZone zone; checkSurface(); SAL_INFO("vcl.skia", "getbitmap(" << this << "): " << Point(nX, nY) << "/" << Size(nWidth, nHeight)); @@ -973,6 +984,7 @@ std::shared_ptr SkiaSalGraphicsImpl::getBitmap(long nX, long nY, long Color SkiaSalGraphicsImpl::getPixel(long nX, long nY) { + SkiaZone zone; checkSurface(); SAL_INFO("vcl.skia", "getpixel(" << this << "): " << Point(nX, nY)); mSurface->getCanvas()->flush(); diff --git a/vcl/skia/salbmp.cxx b/vcl/skia/salbmp.cxx index b321f4c3f6f7..c354b1e34e4b 100644 --- a/vcl/skia/salbmp.cxx +++ b/vcl/skia/salbmp.cxx @@ -34,6 +34,7 @@ #include #include +#include #ifdef DBG_UTIL #include @@ -331,6 +332,7 @@ bool SkiaSalBitmap::ScalingSupported() const { return true; } bool SkiaSalBitmap::Scale(const double& rScaleX, const double& rScaleY, BmpScaleFlag nScaleFlag) { + SkiaZone zone; #ifdef DBG_UTIL assert(mWriteAccessCount == 0); #endif @@ -399,6 +401,7 @@ SkBitmap SkiaSalBitmap::GetAsSkBitmap() const EnsureBitmapData(); if (!mBitmap.isNull()) return mBitmap; + SkiaZone zone; SkBitmap bitmap; if (mBuffer) { @@ -452,6 +455,7 @@ const sk_sp& SkiaSalBitmap::GetSkImage() const #endif if (mImage) return mImage; + SkiaZone zone; sk_sp surface = SkiaHelper::createSkSurface(mSize); assert(surface); SkPaint paint; @@ -469,6 +473,7 @@ const sk_sp& SkiaSalBitmap::GetAlphaSkImage() const #endif if (mAlphaImage) return mAlphaImage; + SkiaZone zone; // TODO can we convert directly mImage -> mAlphaImage? EnsureBitmapData(); SkBitmap alphaBitmap; @@ -538,6 +543,7 @@ void SkiaSalBitmap::EnsureBitmapData() return; if (!mImage) return; + SkiaZone zone; if (!CreateBitmapData()) abort(); if (!mBitmap.isNull()) @@ -627,6 +633,7 @@ void SkiaSalBitmap::EnsureBitmapUniqueData() void SkiaSalBitmap::ResetSkImages() { + SkiaZone zone; mAlphaImage.reset(); mImage.reset(); } diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx index aec2f22e5bb3..0d459be03722 100644 --- a/vcl/skia/win/gdiimpl.cxx +++ b/vcl/skia/win/gdiimpl.cxx @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -27,6 +28,7 @@ WinSkiaSalGraphicsImpl::WinSkiaSalGraphicsImpl(WinSalGraphics& rGraphics, void WinSkiaSalGraphicsImpl::createWindowContext() { + SkiaZone zone; // When created, Init() gets called with size (0,0), which is invalid size // for Skia. Creating the actual surface is delayed, so the size should be always // valid here, but better check. @@ -49,6 +51,7 @@ void WinSkiaSalGraphicsImpl::createWindowContext() void WinSkiaSalGraphicsImpl::DeInit() { + SkiaZone zone; SkiaSalGraphicsImpl::DeInit(); mWindowContext.reset(); } @@ -57,6 +60,7 @@ void WinSkiaSalGraphicsImpl::freeResources() {} void WinSkiaSalGraphicsImpl::performFlush() { + SkiaZone zone; mPendingPixelsToFlush = 0; if (mWindowContext) mWindowContext->swapBuffers(); @@ -158,6 +162,7 @@ std::unique_ptr SkiaCompatibleDC::getAsMaskTexture() cons sk_sp SkiaCompatibleDC::getAsMaskImage() const { + SkiaZone zone; // mpData is in the BGRA format, with A unused (and set to 0), and RGB are grey, // so convert it to Skia format, then to 8bit and finally use as alpha mask SkBitmap tmpBitmap; @@ -200,6 +205,7 @@ sk_sp SkiaCompatibleDC::getAsMaskImage() const sk_sp SkiaCompatibleDC::getAsImage() const { + SkiaZone zone; SkBitmap tmpBitmap; if (!tmpBitmap.installPixels(SkImageInfo::Make(maRects.mnSrcWidth, maRects.mnSrcHeight, kBGRA_8888_SkColorType, kUnpremul_SkAlphaType), @@ -225,6 +231,7 @@ sk_sp SkiaCompatibleDC::getAsImage() const sk_sp SkiaCompatibleDC::getAsImageDiff(const SkiaCompatibleDC& white) const { + SkiaZone zone; assert(maRects.mnSrcWidth == white.maRects.mnSrcWidth || maRects.mnSrcHeight == white.maRects.mnSrcHeight); SkBitmap tmpBitmap; diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx index a139a336c7b7..503eeee3fa32 100644 --- a/vcl/skia/x11/gdiimpl.cxx +++ b/vcl/skia/x11/gdiimpl.cxx @@ -22,6 +22,7 @@ #include #include +#include X11SkiaSalGraphicsImpl::X11SkiaSalGraphicsImpl(X11SalGraphics& rParent) : SkiaSalGraphicsImpl(rParent, rParent.GetGeometryProvider()) @@ -40,6 +41,7 @@ void X11SkiaSalGraphicsImpl::Init() void X11SkiaSalGraphicsImpl::createWindowContext() { + SkiaZone zone; sk_app::DisplayParams displayParams; displayParams.fColorType = kN32_SkColorType; sk_app::window_context_factory::XlibWindowInfo winInfo; @@ -99,6 +101,7 @@ bool X11SkiaSalGraphicsImpl::avoidRecreateByResize() const void X11SkiaSalGraphicsImpl::DeInit() { + SkiaZone zone; SkiaSalGraphicsImpl::DeInit(); mWindowContext.reset(); } @@ -107,6 +110,7 @@ void X11SkiaSalGraphicsImpl::freeResources() {} void X11SkiaSalGraphicsImpl::performFlush() { + SkiaZone zone; mPendingPixelsToFlush = 0; // TODO XPutImage() is somewhat inefficient, XShmPutImage() should be preferred. mWindowContext->swapBuffers(); diff --git a/vcl/skia/zone.cxx b/vcl/skia/zone.cxx new file mode 100644 index 000000000000..82b45620ce1c --- /dev/null +++ b/vcl/skia/zone.cxx @@ -0,0 +1,58 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include + +#include +#include +#include + +#include + +/** + * Called from a signal handler or watchdog thread if we get + * a crash or hang in some driver. + */ +void SkiaZone::hardDisable() +{ + // protect ourselves from double calling etc. + static bool bDisabled = false; + if (!bDisabled) + { + bDisabled = true; + + // Disable Skia support + std::shared_ptr xChanges( + comphelper::ConfigurationChanges::create()); + officecfg::Office::Common::VCL::UseSkia::set(false, xChanges); + xChanges->commit(); + + // Force synchronous config write + css::uno::Reference( + css::configuration::theDefaultProvider::get(comphelper::getProcessComponentContext()), + css::uno::UNO_QUERY_THROW) + ->flush(); + } +} + +void SkiaZone::checkDebug(int nUnchanged, const CrashWatchdogTimingsValues& aTimingValues) +{ + SAL_INFO("vcl.watchdog", "Skia watchdog - unchanged " + << nUnchanged << " enter count " << enterCount() + << " breakpoints mid: " << aTimingValues.mnDisableEntries + << " max " << aTimingValues.mnAbortAfter); +} + +const CrashWatchdogTimingsValues& SkiaZone::getCrashWatchdogTimingsValues() +{ + static const CrashWatchdogTimingsValues values = { 6, 20 }; /* 1.5s, 5s */ + return values; +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/source/app/svmain.cxx b/vcl/source/app/svmain.cxx index 207a6ff4eaad..6c3cd630561c 100644 --- a/vcl/source/app/svmain.cxx +++ b/vcl/source/app/svmain.cxx @@ -88,6 +88,7 @@ #include #include +#include #include #include @@ -123,6 +124,10 @@ static oslSignalAction VCLExceptionSignal_impl( void* /*pData*/, oslSignalInfo* if (OpenGLZone::isInZone()) OpenGLZone::hardDisable(); #endif +#if HAVE_FEATURE_SKIA + if (SkiaZone::isInZone()) + SkiaZone::hardDisable(); +#endif #if HAVE_FEATURE_OPENCL if (OpenCLZone::isInZone()) { diff --git a/vcl/source/app/watchdog.cxx b/vcl/source/app/watchdog.cxx index 795501ec45ac..bd8b28db89e6 100644 --- a/vcl/source/app/watchdog.cxx +++ b/vcl/source/app/watchdog.cxx @@ -17,6 +17,7 @@ #include #include #include +#include #include @@ -107,12 +108,18 @@ void WatchdogThread::execute() #if HAVE_FEATURE_OPENGL WatchdogHelper::setLastEnters(); #endif +#if HAVE_FEATURE_SKIA + WatchdogHelper::setLastEnters(); +#endif gpWatchdogExit->wait(&aQuarterSecond); #if HAVE_FEATURE_OPENGL WatchdogHelper::check(); #endif +#if HAVE_FEATURE_SKIA + WatchdogHelper::check(); +#endif } while (!gpWatchdogExit->check()); } -- cgit v1.2.3