[Libreoffice-commits] core.git: sc/inc sc/source
Michael Meeks
michael.meeks at collabora.com
Thu Nov 23 22:16:04 UTC 2017
sc/inc/document.hxx | 79 ++++++++++++++++++++++++++++--------
sc/source/core/data/documen2.cxx | 5 +-
sc/source/core/data/document.cxx | 27 ------------
sc/source/core/data/formulacell.cxx | 2
4 files changed, 66 insertions(+), 47 deletions(-)
New commits:
commit e4cd5707809042eec435fd24be8729e87e350d1a
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Fri Nov 17 17:36:38 2017 +0000
Document, and simplify the two uses of ScMutationGuard.
Creates ScMutationDisable - used to ensure that no core data
structure is mutated below this guard in any thread. This can
also be used to disable access in the same thread now.
Change-Id: I7e4e98d8ff986490ccd5064b3b9af56acd877b49
Reviewed-on: https://gerrit.libreoffice.org/45119
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx
index c4db749eab6a..bb7bad7652b2 100644
--- a/sc/inc/document.hxx
+++ b/sc/inc/document.hxx
@@ -294,14 +294,11 @@ struct ScDocumentThreadSpecific
void MergeBackIntoNonThreadedData(ScDocumentThreadSpecific& rNonThreadedData);
};
-enum class ScMutationGuardFlags
+/// Enumeration to determine which pieces of the code should not be mutated when set.
+enum ScMutationGuardFlags
{
// Bit mask bits
- CORE = 0x0001,
- FORMULA = 0x0002,
- RECURSIVE_INTERPRET = 0x0004,
- // How many bits there are
- N = 3
+ CORE = 0x0001, /// Core calc data structures should not be mutated
};
class ScDocument
@@ -330,7 +327,8 @@ friend class sc::ColumnSpanSet;
friend class sc::EditTextIterator;
friend class sc::FormulaGroupAreaListener;
friend class sc::TableColumnBlockPositionSet;
-friend class ScMutationGuard;
+friend struct ScMutationGuard;
+friend struct ScMutationDisable;
typedef std::vector<ScTable*> TableContainer;
@@ -536,7 +534,7 @@ private:
bool mbTrackFormulasPending : 1;
bool mbFinalTrackFormulas : 1;
- std::recursive_mutex maMutationGuard[static_cast<std::size_t>(ScMutationGuardFlags::N)];
+ size_t mnMutationGuardFlags;
public:
bool IsCellInChangeTrack(const ScAddress &cell,Color *pColCellBorder);
@@ -550,7 +548,7 @@ public:
// number formatter
public:
SC_DLLPUBLIC ScDocument( ScDocumentMode eMode = SCDOCMODE_DOCUMENT,
- SfxObjectShell* pDocShell = nullptr );
+ SfxObjectShell* pDocShell = nullptr );
SC_DLLPUBLIC ~ScDocument();
void SetName( const OUString& r ) { aDocName = r; }
@@ -2460,17 +2458,64 @@ private:
typedef std::unique_ptr<ScDocument, o3tl::default_delete<ScDocument>> ScDocumentUniquePtr;
-class ScMutationGuard
+/**
+ * Instantiate this to ensure that subsequent modification of
+ * the document will cause an assertion failure while this is
+ * in-scope.
+ */
+struct ScMutationDisable
{
-public:
- ScMutationGuard(ScDocument* pDocument, ScMutationGuardFlags nFlags);
- ~ScMutationGuard();
-
-private:
- ScDocument* const mpDocument;
- const ScMutationGuardFlags mnFlags;
+ ScMutationDisable(ScDocument* pDocument, ScMutationGuardFlags nFlags)
+ {
+#ifndef NDEBUG
+ mpDocument = pDocument;
+ mnFlagRestore = pDocument->mnMutationGuardFlags;
+ assert((mnFlagRestore & nFlags) == 0);
+ mpDocument->mnMutationGuardFlags |= static_cast<size_t>(nFlags);
+#else
+ (void)pDocument; (void)nFlags;
+#endif
+ }
+#ifndef NDEBUG
+ ~ScMutationDisable()
+ {
+ mpDocument->mnMutationGuardFlags = mnFlagRestore;
+ }
+ size_t mnFlagRestore;
+ ScDocument* mpDocument;
+#endif
};
+/**
+ * A pretty assertion that checks that the relevant bits in
+ * the @nFlags are not set on the document at entry and exit.
+ *
+ * Its primary use is for debugging threading. As such, an
+ * @ScMutationDisable is created to forbid mutation, and this
+ * condition is then asserted on at prominent sites that
+ * mutate @nFlags.
+ */
+struct ScMutationGuard
+{
+ ScMutationGuard(ScDocument* pDocument, ScMutationGuardFlags nFlags)
+ {
+#ifndef NDEBUG
+ mpDocument = pDocument;
+ mnFlags = static_cast<size_t>(nFlags);
+ assert((mpDocument->mnMutationGuardFlags & mnFlags) == 0);
+#else
+ (void)pDocument; (void)nFlags;
+#endif
+ }
+#ifndef NDEBUG
+ ~ScMutationGuard()
+ {
+ assert((mpDocument->mnMutationGuardFlags & mnFlags) == 0);
+ }
+ size_t mnFlags;
+ ScDocument* mpDocument;
+#endif
+};
#endif
diff --git a/sc/source/core/data/documen2.cxx b/sc/source/core/data/documen2.cxx
index 9bf0fcf73194..f2e0c7d35b67 100644
--- a/sc/source/core/data/documen2.cxx
+++ b/sc/source/core/data/documen2.cxx
@@ -221,7 +221,8 @@ ScDocument::ScDocument( ScDocumentMode eMode, SfxObjectShell* pDocShell ) :
mnNamedRangesLockCount(0),
mbUseEmbedFonts(false),
mbTrackFormulasPending(false),
- mbFinalTrackFormulas(false)
+ mbFinalTrackFormulas(false),
+ mnMutationGuardFlags(0)
{
SetStorageGrammar( formula::FormulaGrammar::GRAM_STORAGE_DEFAULT);
@@ -485,7 +486,7 @@ void ScDocument::InitClipPtrs( ScDocument* pSourceDoc )
if ( pSourceValid )
pValidationList = new ScValidationDataList(this, *pSourceValid);
- // store Links in Stream
+ // store Links in Stream
delete pClipData;
if (pSourceDoc->GetDocLinkManager().hasDdeLinks())
{
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index 019bfcb99f4a..a585c0342774 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -6742,33 +6742,6 @@ void ScDocument::SetAutoNameCache( ScAutoNameCache* pCache )
pAutoNameCache = pCache;
}
-ScMutationGuard::ScMutationGuard(ScDocument* pDocument, ScMutationGuardFlags nFlags) :
- mpDocument(pDocument),
- mnFlags(nFlags)
-{
- (void) mpDocument;
- for (unsigned b = 0; b < static_cast<std::size_t>(ScMutationGuardFlags::N); b++)
- {
- if (static_cast<std::size_t>(mnFlags) & (static_cast<std::size_t>(1) << b))
- {
- assert(mpDocument->maMutationGuard[b].try_lock());
- }
- }
-}
-
-ScMutationGuard::~ScMutationGuard()
-{
-#ifndef NDEBUG
- for (unsigned b = 0; b < static_cast<std::size_t>(ScMutationGuardFlags::N); b++)
- {
- if (static_cast<std::size_t>(mnFlags) & (static_cast<std::size_t>(1) << b))
- {
- mpDocument->maMutationGuard[b].unlock();
- }
- }
-#endif
-}
-
thread_local ScDocumentThreadSpecific ScDocument::maThreadSpecific;
ScRecursionHelper& ScDocument::GetRecursionHelper()
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 3aaf87a1e629..79b51cc9d25c 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -4416,7 +4416,7 @@ bool ScFormulaCell::InterpretFormulaGroup()
assert(!pDocument->mbThreadedGroupCalcInProgress);
pDocument->mbThreadedGroupCalcInProgress = true;
- ScMutationGuard aGuard(pDocument, ScMutationGuardFlags::CORE);
+ ScMutationDisable aGuard(pDocument, ScMutationGuardFlags::CORE);
// Start nThreadCount new threads
std::shared_ptr<comphelper::ThreadTaskTag> aTag = comphelper::ThreadPool::createThreadTaskTag();
More information about the Libreoffice-commits
mailing list