vcl/inc/qt5/QtTransferable.hxx | 10 +++++++++-
vcl/qt5/QtClipboard.cxx | 2 +-
vcl/qt5/QtTransferable.cxx | 8 ++++++++
3 files changed, 18 insertions(+), 2 deletions(-)
New commits:
commit 8fbff7fa22cc50132f09442e338fc71434c4a77a
Author: Michael Weghorn <m.wegh...@posteo.de>
AuthorDate: Fri Apr 26 15:04:24 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Tue Apr 30 11:07:08 2024 +0200
qt: Guard clipboard mime data with SolarMutex
Most of the access to the QtClipboardTransferable
mime data happens exclusively on the main thread,
with the solar mutex held.
However, `mimeData()`, called from `QtClipboard::getContents`
didn't ensure that yet, so as Michael Stahl pointed out in [1],
commit 1db5b87fe69c2375f1d66974dafcd563303c76db
Author: Michael Weghorn <m.wegh...@posteo.de>
Date: Tue Feb 13 13:23:17 2024 +0100
tdf#156562 qt: Sync with system clipboard content if necessary
introduced a data race by allowing to set new mime data.
Introduce a new
`QtClipboardTransferable::hasMimeData(const QMimeData* pMimeData)`
that guards access to the mime data with the solar mutext as well
and use that instead, so all access to the `QtClipboardTransferable`
mime data is now guarded by the solar mutex.
Also add an explicit note for the mime data getter/setter in the
`QtTransferable` base class that subclasses allowing to update
mime data are responsible for preventing data races.
[1] https://gerrit.libreoffice.org/c/core/+/166141/comment/fe75f418_40c1b622
Change-Id: I01dbbb0b37a4c6ad06b4d3001ecce8b0260eb32e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166750
Reviewed-by: Michael Weghorn <m.wegh...@posteo.de>
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
(cherry picked from commit 621cfc0e4120ab2b381b54268fe39bd19257df9b)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166892
diff --git a/vcl/inc/qt5/QtTransferable.hxx b/vcl/inc/qt5/QtTransferable.hxx
index c58490e90460..5687fa06df52 100644
--- a/vcl/inc/qt5/QtTransferable.hxx
+++ b/vcl/inc/qt5/QtTransferable.hxx
@@ -40,12 +40,17 @@ protected:
* Since data flavors supported by this class depend on the mime data,
* results from previous calls to the public methods of this
* class are no longer valid after setting new mime data using this
method.
+ *
+ * Subclasses that set new mime data must ensure that no data race exists
+ * on m_pMimeData.
+ * (For the current only subclass doing so, QtClipboardTransferable, all
access
+ * to m_pMimeData happens with the SolarMutex held.)
*/
void setMimeData(const QMimeData* pMimeData) { m_pMimeData = pMimeData; }
+ const QMimeData* mimeData() const { return m_pMimeData; }
public:
QtTransferable(const QMimeData* pMimeData);
- const QMimeData* mimeData() const { return m_pMimeData; }
css::uno::Sequence<css::datatransfer::DataFlavor> SAL_CALL
getTransferDataFlavors() override;
sal_Bool SAL_CALL isDataFlavorSupported(const
css::datatransfer::DataFlavor& rFlavor) override;
@@ -74,6 +79,9 @@ class QtClipboardTransferable final : public QtTransferable
public:
explicit QtClipboardTransferable(const QClipboard::Mode aMode, const
QMimeData* pMimeData);
+ // whether pMimeData are the current mime data
+ bool hasMimeData(const QMimeData* pMimeData) const;
+
// these are the same then QtTransferable, except they go through
RunInMainThread
css::uno::Sequence<css::datatransfer::DataFlavor> SAL_CALL
getTransferDataFlavors() override;
sal_Bool SAL_CALL isDataFlavorSupported(const
css::datatransfer::DataFlavor& rFlavor) override;
diff --git a/vcl/qt5/QtClipboard.cxx b/vcl/qt5/QtClipboard.cxx
index e9eb476fb253..ea05784bbfb2 100644
--- a/vcl/qt5/QtClipboard.cxx
+++ b/vcl/qt5/QtClipboard.cxx
@@ -103,7 +103,7 @@ css::uno::Reference<css::datatransfer::XTransferable>
QtClipboard::getContents()
{
const auto* pTrans =
dynamic_cast<QtClipboardTransferable*>(m_aContents.get());
assert(pTrans);
- if (pTrans && pTrans->mimeData() == pMimeData)
+ if (pTrans && pTrans->hasMimeData(pMimeData))
return m_aContents;
}
diff --git a/vcl/qt5/QtTransferable.cxx b/vcl/qt5/QtTransferable.cxx
index a6902824ab3a..1aec5da27843 100644
--- a/vcl/qt5/QtTransferable.cxx
+++ b/vcl/qt5/QtTransferable.cxx
@@ -13,6 +13,7 @@
#include <comphelper/sequence.hxx>
#include <sal/log.hxx>
#include <o3tl/string_view.hxx>
+#include <tools/debug.hxx>
#include <QtWidgets/QApplication>
@@ -173,10 +174,17 @@ void
QtClipboardTransferable::ensureConsistencyWithSystemClipboard()
{
SAL_WARN("vcl.qt", "In flight clipboard change detected - updating
mime data with current "
"clipboard contents.");
+ DBG_TESTSOLARMUTEX();
setMimeData(pCurrentClipboardData);
}
}
+bool QtClipboardTransferable::hasMimeData(const QMimeData* pMimeData) const
+{
+ SolarMutexGuard aGuard;
+ return QtTransferable::mimeData() == pMimeData;
+}
+
css::uno::Any SAL_CALL
QtClipboardTransferable::getTransferData(const css::datatransfer::DataFlavor&
rFlavor)
{
