From e1402abc5c6d08ae541ca6ef31b017034232d4cf Mon Sep 17 00:00:00 2001 From: "Armin Le Grand (allotropia)" Date: Tue, 2 Jan 2024 16:04:14 +0100 Subject: tdf#158913 secure Primitive 'visit' using mutex See text in tdf task for detailed description. Change-Id: I4677c9f2ecfdcb760d05fe916a2aa2dd25ad40b6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161543 Tested-by: Jenkins Reviewed-by: Armin Le Grand --- .../BufferedDecompositionGroupPrimitive2D.cxx | 54 ++++++++++++++-------- .../BufferedDecompositionPrimitive2D.cxx | 54 ++++++++++++++-------- 2 files changed, 72 insertions(+), 36 deletions(-) (limited to 'drawinglayer') diff --git a/drawinglayer/source/primitive2d/BufferedDecompositionGroupPrimitive2D.cxx b/drawinglayer/source/primitive2d/BufferedDecompositionGroupPrimitive2D.cxx index 96fd72eb25ca..a24816ad5f3a 100644 --- a/drawinglayer/source/primitive2d/BufferedDecompositionGroupPrimitive2D.cxx +++ b/drawinglayer/source/primitive2d/BufferedDecompositionGroupPrimitive2D.cxx @@ -70,32 +70,38 @@ BufferedDecompositionGroupPrimitive2D::getBuffered2DDecomposition() const void BufferedDecompositionGroupPrimitive2D::setBuffered2DDecomposition(Primitive2DContainer&& rNew) { - if (0 != maCallbackSeconds) + if (0 == maCallbackSeconds) { - if (maCallbackTimer.is()) + // no flush used, just set + maBuffered2DDecomposition = std::move(rNew); + return; + } + + if (maCallbackTimer.is()) + { + if (rNew.empty()) { - if (rNew.empty()) - { - // stop timer - maCallbackTimer->stop(); - } - else - { - // decomposition changed, touch - maCallbackTimer->setRemainingTime(salhelper::TTimeValue(maCallbackSeconds, 0)); - if (!maCallbackTimer->isTicking()) - maCallbackTimer->start(); - } + // stop timer + maCallbackTimer->stop(); } - else if (!rNew.empty()) + else { - // decomposition defined/set/changed, init & start timer - maCallbackTimer.set(new LocalCallbackTimer(*this)); + // decomposition changed, touch maCallbackTimer->setRemainingTime(salhelper::TTimeValue(maCallbackSeconds, 0)); - maCallbackTimer->start(); + if (!maCallbackTimer->isTicking()) + maCallbackTimer->start(); } } + else if (!rNew.empty()) + { + // decomposition defined/set/changed, init & start timer + maCallbackTimer.set(new LocalCallbackTimer(*this)); + maCallbackTimer->setRemainingTime(salhelper::TTimeValue(maCallbackSeconds, 0)); + maCallbackTimer->start(); + } + // tdf#158913 need to secure change when flush/multithreading is in use + std::lock_guard Guard(maCallbackLock); maBuffered2DDecomposition = std::move(rNew); } @@ -103,6 +109,7 @@ BufferedDecompositionGroupPrimitive2D::BufferedDecompositionGroupPrimitive2D( Primitive2DContainer&& aChildren) : GroupPrimitive2D(std::move(aChildren)) , maCallbackTimer() + , maCallbackLock() , maCallbackSeconds(0) { } @@ -129,6 +136,17 @@ void BufferedDecompositionGroupPrimitive2D::get2DDecomposition( std::move(aNewSequence)); } + if (0 == maCallbackSeconds) + { + // no flush/multithreading is in use, just call + rVisitor.visit(getBuffered2DDecomposition()); + return; + } + + // tdf#158913 need to secure 'visit' when flush/multithreading is in use, + // so that the local non-ref-Counted instance of the decomposition gets not + // manipulated (e.g. deleted) + std::lock_guard Guard(maCallbackLock); rVisitor.visit(getBuffered2DDecomposition()); } diff --git a/drawinglayer/source/primitive2d/BufferedDecompositionPrimitive2D.cxx b/drawinglayer/source/primitive2d/BufferedDecompositionPrimitive2D.cxx index c7f3b78477b0..22b6450e7f2d 100644 --- a/drawinglayer/source/primitive2d/BufferedDecompositionPrimitive2D.cxx +++ b/drawinglayer/source/primitive2d/BufferedDecompositionPrimitive2D.cxx @@ -69,38 +69,45 @@ const Primitive2DContainer& BufferedDecompositionPrimitive2D::getBuffered2DDecom void BufferedDecompositionPrimitive2D::setBuffered2DDecomposition(Primitive2DContainer&& rNew) { - if (0 != maCallbackSeconds) + if (0 == maCallbackSeconds) { - if (maCallbackTimer.is()) + // no flush used, just set + maBuffered2DDecomposition = std::move(rNew); + return; + } + + if (maCallbackTimer.is()) + { + if (rNew.empty()) { - if (rNew.empty()) - { - // stop timer - maCallbackTimer->stop(); - } - else - { - // decomposition changed, touch - maCallbackTimer->setRemainingTime(salhelper::TTimeValue(maCallbackSeconds, 0)); - if (!maCallbackTimer->isTicking()) - maCallbackTimer->start(); - } + // stop timer + maCallbackTimer->stop(); } - else if (!rNew.empty()) + else { - // decomposition defined/set/changed, init & start timer - maCallbackTimer.set(new LocalCallbackTimer(*this)); + // decomposition changed, touch maCallbackTimer->setRemainingTime(salhelper::TTimeValue(maCallbackSeconds, 0)); - maCallbackTimer->start(); + if (!maCallbackTimer->isTicking()) + maCallbackTimer->start(); } } + else if (!rNew.empty()) + { + // decomposition defined/set/changed, init & start timer + maCallbackTimer.set(new LocalCallbackTimer(*this)); + maCallbackTimer->setRemainingTime(salhelper::TTimeValue(maCallbackSeconds, 0)); + maCallbackTimer->start(); + } + // tdf#158913 need to secure change when flush/multithreading is in use + std::lock_guard Guard(maCallbackLock); maBuffered2DDecomposition = std::move(rNew); } BufferedDecompositionPrimitive2D::BufferedDecompositionPrimitive2D() : maBuffered2DDecomposition() , maCallbackTimer() + , maCallbackLock() , maCallbackSeconds(0) , mnTransparenceForShadow(0) { @@ -128,6 +135,17 @@ void BufferedDecompositionPrimitive2D::get2DDecomposition( std::move(aNewSequence)); } + if (0 == maCallbackSeconds) + { + // no flush/multithreading is in use, just call + rVisitor.visit(getBuffered2DDecomposition()); + return; + } + + // tdf#158913 need to secure 'visit' when flush/multithreading is in use, + // so that the local non-ref-Counted instance of the decomposition gets not + // manipulated (e.g. deleted) + std::lock_guard Guard(maCallbackLock); rVisitor.visit(getBuffered2DDecomposition()); } -- cgit