[Libreoffice-commits] core.git: sw/inc sw/source

Stephan Bergmann (via logerrit) logerrit at kemper.freedesktop.org
Thu Dec 3 15:54:15 UTC 2020


 sw/inc/doc.hxx                |    9 ++++++---
 sw/source/core/doc/docfmt.cxx |   13 ++++++-------
 sw/source/core/doc/docnew.cxx |   14 ++++++++++++--
 3 files changed, 24 insertions(+), 12 deletions(-)

New commits:
commit a18217c04ba2c927669bd0512eaf57a9f07ba9c4
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Thu Dec 3 14:14:49 2020 +0100
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Thu Dec 3 16:53:35 2020 +0100

    Just use a mutex for access to SwDoc::mpNumberFormatter
    
    ...reverting the use of std::atomic and comphelper::doubleCheckedInit introduced
    in 977a98c5729b4301c11cab1a421d4e6f2758e41e "crashtesting: intermittent threaded
    crash".  I have once seen UITest_writer_tests deadlock with
    
    > Thread 7 (Thread 0x7faad204f640 (LWP 359621) "cppu_threadpool"):
    > #0  __lll_lock_wait (futex=0x6040000a7450, private=0) at /usr/src/debug/glibc-2.32-20-g5c36293f06/nptl/lowlevellock.c:52
    > #1  0x00007fab5a7a57f1 in __GI___pthread_mutex_lock (mutex=0x6040000a7450) at /usr/src/debug/glibc-2.32-20-g5c36293f06/nptl/pthread_mutex_lock.c:115
    > #2  0x00007fab5c0c9bb7 in osl_acquireMutex(oslMutex) (pMutex=0x6040000a7450) at /data/sbergman/lo-san/core/sal/osl/unx/mutex.cxx:100
    > #3  0x00007fab53d4cd6a in osl::Mutex::acquire() (this=0x7fab545a57c0 <rtl::Static<osl::Mutex, (anonymous namespace)::theImplHelperInitMutex>::get()::instance>) at /data/sbergman/lo-san/core/include/osl/mutex.hxx:57
    > #4  0x00007fab53dbb4ff in osl::Guard<osl::Mutex>::Guard(osl::Mutex&) (this=0x7faad0f33820, t=...) at /data/sbergman/lo-san/core/include/osl/mutex.hxx:135
    > #5  0x00007fab53e51959 in cppu::getTypeEntries(cppu::class_data*) (cd=0x7faaec3567d0 <cppu::detail::ImplClassData<cppu::WeakImplHelper<com::sun::star::i18n::XNumberFormatCode, com::sun::star::lang::XServiceInfo>, com::sun::star::i18n::XNumberFormatCode, com::sun::star::lang::XServiceInfo>::operator()()::s_cd>) at /data/sbergman/lo-san/core/cppuhelper/source/implbase_ex.cxx:82
    > #6  0x00007fab53e4cee6 in cppu::queryDeepNoXInterface(_typelib_TypeDescriptionReference const*, cppu::class_data*, void*) (pDemandedTDR=0x60f0000116b0, cd=0x7faaec3567d0 <cppu::detail::ImplClassData<cppu::WeakImplHelper<com::sun::star::i18n::XNumberFormatCode, com::sun::star::lang::XServiceInfo>, com::sun::star::i18n::XNumberFormatCode, com::sun::star::lang::XServiceInfo>::operator()()::s_cd>, that=0x60f0000117a0) at /data/sbergman/lo-san/core/cppuhelper/source/implbase_ex.cxx:166
    > #7  0x00007fab53e4f3b3 in cppu::WeakImplHelper_query(com::sun::star::uno::Type const&, cppu::class_data*, void*, cppu::OWeakObject*) (rType=invalid uno::Type, cd=0x7faaec3567d0 <cppu::detail::ImplClassData<cppu::WeakImplHelper<com::sun::star::i18n::XNumberFormatCode, com::sun::star::lang::XServiceInfo>, com::sun::star::i18n::XNumberFormatCode, com::sun::star::lang::XServiceInfo>::operator()()::s_cd>, that=0x60f0000117a0, pBase=0x60f0000117a0) at /data/sbergman/lo-san/core/cppuhelper/source/implbase_ex.cxx:294
    > #8  0x00007faaec053845 in cppu::WeakImplHelper<com::sun::star::i18n::XNumberFormatCode, com::sun::star::lang::XServiceInfo>::queryInterface(com::sun::star::uno::Type const&) (this=0x60f0000117a0, aType=invalid uno::Type) at /data/sbergman/lo-san/core/include/cppuhelper/implbase.hxx:111
    > #9  0x00007fab41f393c0 in com::sun::star::uno::BaseReference::iquery(com::sun::star::uno::XInterface*, com::sun::star::uno::Type const&) (pInterface=0x60f0000117a0, rType=invalid uno::Type) at /data/sbergman/lo-san/core/include/com/sun/star/uno/Reference.hxx:55
    > #10 0x00007fab42a09ff5 in com::sun::star::uno::Reference<com::sun::star::i18n::XNumberFormatCode>::iquery(com::sun::star::uno::XInterface*) (pInterface=0x60f0000117a0) at /data/sbergman/lo-san/core/include/com/sun/star/uno/Reference.hxx:70
    > #11 0x00007fab42a08fae in com::sun::star::uno::Reference<com::sun::star::i18n::XNumberFormatCode>::Reference(com::sun::star::uno::BaseReference const&, com::sun::star::uno::UnoReference_Query) (this=0x7faad0de1420, rRef=...) at /data/sbergman/lo-san/core/include/com/sun/star/uno/Reference.hxx:172
    > #12 0x00007fab429d6ec1 in com::sun::star::i18n::NumberFormatMapper::create(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&) (the_context=uno::Reference to (class cppu::(anonymous namespace)::ComponentContext *) 0x611000006710) at /data/sbergman/lo-san/core/workdir/UnoApiHeadersTarget/offapi/normal/com/sun/star/i18n/NumberFormatMapper.hpp:38
    > #13 0x00007fab42b03801 in ImpSvNumberformatScan::ImpSvNumberformatScan(SvNumberFormatter*) (this=0x61a000036c80, pFormatterP=0x616000442580) at /data/sbergman/lo-san/core/svl/source/numbers/zforscan.cxx:132
    > #14 0x00007fab4296fe31 in SvNumberFormatter::ImpConstruct(o3tl::strong_int<unsigned short, LanguageTypeTag>) (this=0x616000442580, eLang=...) at /data/sbergman/lo-san/core/svl/source/numbers/zforlist.cxx:320
    > #15 0x00007fab4296ed9c in SvNumberFormatter::SvNumberFormatter(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, o3tl::strong_int<unsigned short, LanguageTypeTag>) (this=0x616000442580, rxContext=uno::Reference to (class cppu::(anonymous namespace)::ComponentContext *) 0x611000006710, eLang=...) at /data/sbergman/lo-san/core/svl/source/numbers/zforlist.cxx:274
    > #16 0x00007faac3951184 in SwDoc::EnsureNumberFormatter()::$_0::operator()() const (this=0x7faad0a53ba0) at /data/sbergman/lo-san/core/sw/source/core/doc/docfmt.cxx:1728
    > #17 0x00007faac3942b5c in comphelper::doubleCheckedInit<SvNumberFormatter, SwDoc::EnsureNumberFormatter()::$_0, osl::Guard<osl::Mutex>, osl::GetGlobalMutex>(std::atomic<SvNumberFormatter*>&, SwDoc::EnsureNumberFormatter()::$_0, osl::GetGlobalMutex) (pointer=..., function=..., guardCtor=...) at /data/sbergman/lo-san/core/include/comphelper/doublecheckedinit.hxx:53
    > #18 0x00007faac3942984 in SwDoc::EnsureNumberFormatter() (this=0x619000395d80) at /data/sbergman/lo-san/core/sw/source/core/doc/docfmt.cxx:1725
    [...]
    
    acquiring the osl::GetGlobalMutex() in frame 17 and trying to acquire the
    getImplHelperInitMutex() in frame 5, and
    
    > Thread 4 (Thread 0x7faad5a52640 (LWP 359614) "InitUpdateCheck"):
    > #0  __lll_lock_wait (futex=0x7fab5c2e69e0 <globalMutexImpl>, private=0) at /usr/src/debug/glibc-2.32-20-g5c36293f06/nptl/lowlevellock.c:52
    > #1  0x00007fab5a7a57f1 in __GI___pthread_mutex_lock (mutex=0x7fab5c2e69e0 <globalMutexImpl>) at /usr/src/debug/glibc-2.32-20-g5c36293f06/nptl/pthread_mutex_lock.c:115
    > #2  0x00007fab5c0c9bb7 in osl_acquireMutex(oslMutex) (pMutex=0x7fab5c2e69e0 <globalMutexImpl>) at /data/sbergman/lo-san/core/sal/osl/unx/mutex.cxx:100
    > #3  0x00007fa848b547ca in osl::Mutex::acquire() (this=0x7fab5c294960 <osl_getGlobalMutex::globalMutex>) at /data/sbergman/lo-san/core/include/osl/mutex.hxx:57
    > #4  0x00007fa848b52c3a in osl::Guard<osl::Mutex>::Guard(osl::Mutex*) (this=0x7faad467c820, pT_=0x7fab5c294960 <osl_getGlobalMutex::globalMutex>) at /data/sbergman/lo-san/core/include/osl/mutex.hxx:128
    > #5  0x00007fa848b6515d in com::sun::star::lang::cppu_detail_getUnoType(com::sun::star::lang::XTypeProvider const*) () at /data/sbergman/lo-san/core/workdir/UnoApiHeadersTarget/udkapi/normal/com/sun/star/lang/XTypeProvider.hpp:67
    > #6  0x00007fa848b64e49 in cppu::UnoType<com::sun::star::lang::XTypeProvider>::get() () at /data/sbergman/lo-san/core/include/cppu/unotype.hxx:296
    > #7  0x00007fa848b62049 in com::sun::star::lang::XTypeProvider::static_type(void*) () at /data/sbergman/lo-san/core/workdir/UnoApiHeadersTarget/udkapi/normal/com/sun/star/lang/XTypeProvider.hpp:118
    > #8  0x00007fab53e51d46 in cppu::getTypeEntries(cppu::class_data*) (cd=0x7fa848bb4b28 <cppu::detail::ImplClassData<cppu::WeakImplHelper<com::sun::star::deployment::XUpdateInformationProvider, com::sun::star::ucb::XWebDAVCommandEnvironment, com::sun::star::lang::XServiceInfo>, com::sun::star::deployment::XUpdateInformationProvider, com::sun::star::ucb::XWebDAVCommandEnvironment, com::sun::star::lang::XServiceInfo>::operator()()::s_cd>) at /data/sbergman/lo-san/core/cppuhelper/source/implbase_ex.cxx:89
    [...]
    
    acqiring the getImplHelperInitMutex() in frame 8 and trying to acquire the
    osl::GetGlobalMutex() in frame 5.
    
    (While we could have additionally reverted mpNumberFormatter to a unique_ptr,
    all places that would---explicitly or implicitly---reset it would need to do so
    with mNumberFormatterMutex locked, so those places need to be explicit anyway.)
    
    Change-Id: Ide52279e81a5a70b57565a1d11fb099f0c19f5ae
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107167
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/sw/inc/doc.hxx b/sw/inc/doc.hxx
index 1cf15c399293..3183b97f125f 100644
--- a/sw/inc/doc.hxx
+++ b/sw/inc/doc.hxx
@@ -40,8 +40,8 @@
 #include "tblenum.hxx"
 #include "ndarr.hxx"
 #include "ndtyp.hxx"
-#include <atomic>
 #include <memory>
+#include <mutex>
 #include <set>
 #include <tuple>
 #include <unordered_map>
@@ -261,7 +261,9 @@ class SW_DLLPUBLIC SwDoc final
     std::unique_ptr<SwAutoCorrExceptWord> mpACEWord;               /**< For the automated takeover of
                                                    auto-corrected words that are "re-corrected". */
     std::unique_ptr<SwURLStateChanged> mpURLStateChgd;             //< SfxClient for changes in INetHistory
-    std::atomic<SvNumberFormatter*> mpNumberFormatter;             //< NumFormatter for tables / fields
+
+    std::mutex mNumberFormatterMutex;
+    SvNumberFormatter* mpNumberFormatter;             //< NumFormatter for tables / fields
 
     mutable std::unique_ptr<SwNumRuleTable> mpNumRuleTable;     //< List of all named NumRules.
 
@@ -354,7 +356,7 @@ private:
                                 const OUString& rFormula,
                                 std::vector<OUString>& rUsedDBNames );
 
-    void EnsureNumberFormatter();
+    void EnsureNumberFormatter(); // must be called with mNumberFormatterMutex locked
 
     bool UnProtectTableCells( SwTable& rTable );
 
@@ -1408,6 +1410,7 @@ public:
     // Query NumberFormatter.
     SvNumberFormatter* GetNumberFormatter(bool bCreate = true)
     {
+        std::scoped_lock lock(mNumberFormatterMutex);
         if (bCreate)
             EnsureNumberFormatter();
         return mpNumberFormatter;
diff --git a/sw/source/core/doc/docfmt.cxx b/sw/source/core/doc/docfmt.cxx
index c4c8b1a64ebc..45855c20c9b5 100644
--- a/sw/source/core/doc/docfmt.cxx
+++ b/sw/source/core/doc/docfmt.cxx
@@ -28,7 +28,6 @@
 #include <officecfg/Office/Common.hxx>
 #include <osl/diagnose.h>
 #include <svl/zforlist.hxx>
-#include <comphelper/doublecheckedinit.hxx>
 #include <comphelper/processfactory.hxx>
 #include <unotools/configmgr.hxx>
 #include <sal/log.hxx>
@@ -1722,15 +1721,15 @@ SwTableLineFormat* SwDoc::MakeTableLineFormat()
 
 void SwDoc::EnsureNumberFormatter()
 {
-    comphelper::doubleCheckedInit(mpNumberFormatter, []()
+    if (mpNumberFormatter == nullptr)
     {
         LanguageType eLang = LANGUAGE_SYSTEM;
-        SvNumberFormatter* pRet = new SvNumberFormatter(comphelper::getProcessComponentContext(), eLang);
-        pRet->SetEvalDateFormat( NF_EVALDATEFORMAT_FORMAT_INTL );
+        mpNumberFormatter = new SvNumberFormatter(comphelper::getProcessComponentContext(), eLang);
+        mpNumberFormatter->SetEvalDateFormat( NF_EVALDATEFORMAT_FORMAT_INTL );
         if (!utl::ConfigManager::IsFuzzing())
-            pRet->SetYear2000(officecfg::Office::Common::DateFormat::TwoDigitYear::get());
-        return pRet;
-    });
+            mpNumberFormatter->SetYear2000(
+                officecfg::Office::Common::DateFormat::TwoDigitYear::get());
+    };
 }
 
 SwTableNumFormatMerge::SwTableNumFormatMerge( const SwDoc& rSrc, SwDoc& rDest )
diff --git a/sw/source/core/doc/docnew.cxx b/sw/source/core/doc/docnew.cxx
index e656ee85e78d..757e95334011 100644
--- a/sw/source/core/doc/docnew.cxx
+++ b/sw/source/core/doc/docnew.cxx
@@ -17,6 +17,10 @@
  *   the License at http://www.apache.org/licenses/LICENSE-2.0 .
  */
 
+#include <sal/config.h>
+
+#include <mutex>
+
 #include <config_features.h>
 
 #include <o3tl/sorted_vector.hxx>
@@ -584,7 +588,10 @@ SwDoc::~SwDoc()
 
     disposeXForms(); // #i113606#, dispose the XForms objects
 
-    delete mpNumberFormatter.load(); mpNumberFormatter= nullptr;
+    {
+        std::scoped_lock lock(mNumberFormatterMutex);
+        delete mpNumberFormatter; mpNumberFormatter= nullptr;
+    }
     mpFootnoteInfo.reset();
     mpEndNoteInfo.reset();
     mpLineNumberInfo.reset();
@@ -741,7 +748,10 @@ void SwDoc::ClearDoc()
 
     GetDocumentFieldsManager().ClearFieldTypes();
 
-    delete mpNumberFormatter.load(); mpNumberFormatter= nullptr;
+    {
+        std::scoped_lock lock(mNumberFormatterMutex);
+        delete mpNumberFormatter; mpNumberFormatter= nullptr;
+    }
 
     getIDocumentStylePoolAccess().GetPageDescFromPool( RES_POOLPAGE_STANDARD );
     pFirstNd->ChgFormatColl( getIDocumentStylePoolAccess().GetTextCollFromPool( RES_POOLCOLL_STANDARD ));


More information about the Libreoffice-commits mailing list