Skip to content

Commit c72ceae

Browse files
stbergmanncaolanm
authored andcommitted
Revert part of "register caches with ImplSVData"...
...and of "use a different memory allocation strategy for caches" This reverts the sw/source/core/ole/ndole.cxx part of commit 3fc2216 and of commit b8935ee for now, as it causes > ==489880==ERROR: AddressSanitizer: heap-use-after-free on address 0x7c4dc7e22210 at pc 0x7b6d336739b2 bp 0x7ffc09ed2d50 sp 0x7ffc09ed2d48 > READ of size 8 at 0x7c4dc7e22210 thread T0 (kitbroker_003) > #0 in __gnu_cxx::__normal_iterator<SwOLEObj* const*, std::__cxx1998::vector<SwOLEObj*, std::pmr::polymorphic_allocator<SwOLEObj*>>>::__normal_iterator(SwOLEObj* const* const&) at /usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/bits/stl_iterator.h:1059:20 > #1 in std::__cxx1998::vector<SwOLEObj*, std::pmr::polymorphic_allocator<SwOLEObj*>>::begin() const at /usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/bits/stl_vector.h:1009:16 > #2 in std::__cxx1998::vector<SwOLEObj*, std::pmr::polymorphic_allocator<SwOLEObj*>>::empty() const at /usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/bits/stl_vector.h:1224:16 > #3 in (anonymous namespace)::SwOLELRUCache::dropCaches() at core/sw/source/core/ole/ndole.cxx:98:29 > #4 in ImplSVData::dropCaches() at core/vcl/source/app/svdata.cxx:455:43 > #5 in vcl::lok::trimMemory(int) at core/vcl/source/app/svapp.cxx:1800:18 > #6 in lo_trimMemory(_LibreOfficeKit*, int) at core/desktop/source/lib/init.cxx:3403:5 > #7 in Document::trimAfterInactivity() at online/kit/Kit.cpp:1122:17 > #8 in KitSocketPoll::kitPoll(int) at online/kit/Kit.cpp:3045:20 > #9 in SvpSalInstance::ImplYield(bool, bool) at core/vcl/headless/svpinst.cxx:463:31 > #10 in SvpSalInstance::DoYield(bool, bool) at core/vcl/headless/svpinst.cxx:504:21 > #11 in ImplYield(bool, bool) at core/vcl/source/app/svapp.cxx:389:48 > #12 in Application::Yield() at core/vcl/source/app/svapp.cxx:492:5 > #13 in Application::Execute() at core/vcl/source/app/svapp.cxx:364:13 > #14 in desktop::Desktop::Main() at core/desktop/source/app/app.cxx:1680:13 > #15 in ImplSVMain() at core/vcl/source/app/svmain.cxx:228:35 > #16 in SVMain() at core/vcl/source/app/svmain.cxx:246:12 > #17 in soffice_main at core/desktop/source/app/sofficemain.cxx:121:12 > #18 in lo_runLoop(_LibreOfficeKit*, int (*)(void*, int), void (*)(void*), void*) at core/desktop/source/lib/init.cxx:7849:9 > #19 in lokit_main(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool, bool, bool, bool, bool, bool, unsigned long) at online/kit/Kit.cpp:3928:16 > #20 in createLibreOfficeKit(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool, bool)::$_0::operator()() const at online/kit/ForKit.cpp:553:13 > #21 in void std::__invoke_impl<void, createLibreOfficeKit(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool, bool)::$_0&>(std::__invoke_other, createLibreOfficeKit(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool, bool)::$_0&) at /usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/bits/invoke.h:63:14 > #22 in std::enable_if<is_invocable_r_v<void, createLibreOfficeKit(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool, bool)::$_0&>, void>::type std::__invoke_r<void, createLibreOfficeKit(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool, bool)::$_0&>(createLibreOfficeKit(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool, bool)::$_0&) at /usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/bits/invoke.h:113:2 > #23 in std::_Function_handler<void (), createLibreOfficeKit(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool, bool)::$_0>::_M_invoke(std::_Any_data const&) at /usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/bits/std_function.h:292:9 > #24 in std::function<void ()>::operator()() const at /usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/bits/std_function.h:593:9 > #25 in forkKit(std::function<void ()> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::function<void (int)> const&) at online/kit/ForKit.cpp:496:9 > #26 in createLibreOfficeKit(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool, bool) at online/kit/ForKit.cpp:573:20 > #27 in forkLibreOfficeKit(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool) at online/kit/ForKit.cpp:698:39 > #28 in forkit_main(int, char**) at online/kit/ForKit.cpp:1100:17 > #29 in __libc_start_call_main at <null> Change-Id: I8fdc88ca965e27c16b92df1c05e95eca23ab57b6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190379 Reviewed-by: Caolán McNamara <[email protected]> Tested-by: Caolán McNamara <[email protected]> Tested-by: Jenkins CollaboraOffice <[email protected]>
1 parent f7ba8de commit c72ceae

File tree

1 file changed

+18
-50
lines changed

1 file changed

+18
-50
lines changed

sw/source/core/ole/ndole.cxx

Lines changed: 18 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
#include <sfx2/linkmgr.hxx>
3434
#include <unotools/configitem.hxx>
3535
#include <utility>
36-
#include <vcl/dropcache.hxx>
3736
#include <vcl/outdev.hxx>
3837
#include <fmtanchr.hxx>
3938
#include <frmfmt.hxx>
@@ -58,7 +57,7 @@
5857
#include <svx/unopage.hxx>
5958
#include <comphelper/threadpool.hxx>
6059
#include <atomic>
61-
#include <vector>
60+
#include <deque>
6261
#include <libxml/xmlwriter.h>
6362
#include <osl/diagnose.h>
6463
#include <flyfrm.hxx>
@@ -71,39 +70,14 @@ namespace {
7170

7271
class SwOLELRUCache
7372
: private utl::ConfigItem
74-
, public CacheOwner
7573
{
7674
private:
77-
#if defined __cpp_lib_memory_resource
78-
typedef std::pmr::vector<SwOLEObj*> vector_t;
79-
#else
80-
typedef std::vector<SwOLEObj*> vector_t;
81-
#endif
82-
vector_t m_OleObjects;
75+
std::deque<SwOLEObj *> m_OleObjects;
8376
sal_Int32 m_nLRU_InitSize;
8477
static uno::Sequence< OUString > GetPropertyNames();
8578

8679
virtual void ImplCommit() override;
8780

88-
void tryShrinkCacheTo(sal_Int32 nVal);
89-
90-
virtual OUString getCacheName() const override
91-
{
92-
return "SwOLELRUCache";
93-
}
94-
95-
virtual bool dropCaches() override
96-
{
97-
tryShrinkCacheTo(0);
98-
return m_OleObjects.empty();
99-
}
100-
101-
virtual void dumpState(rtl::OStringBuffer& rState) override
102-
{
103-
rState.append("\nSwOLELRUCache:\t");
104-
rState.append(static_cast<sal_Int32>(m_OleObjects.size()));
105-
}
106-
10781
public:
10882
SwOLELRUCache();
10983

@@ -1310,9 +1284,6 @@ void SwOLEObj::dumpAsXml(xmlTextWriterPtr pWriter) const
13101284

13111285
SwOLELRUCache::SwOLELRUCache()
13121286
: utl::ConfigItem(u"Office.Common/Cache"_ustr)
1313-
#if defined __cpp_lib_memory_resource
1314-
, m_OleObjects(&GetMemoryResource())
1315-
#endif
13161287
, m_nLRU_InitSize( 20 )
13171288
{
13181289
EnableNotification( GetPropertyNames() );
@@ -1334,23 +1305,6 @@ void SwOLELRUCache::ImplCommit()
13341305
{
13351306
}
13361307

1337-
void SwOLELRUCache::tryShrinkCacheTo(sal_Int32 nVal)
1338-
{
1339-
// size of cache has been changed
1340-
sal_Int32 nCount = m_OleObjects.size();
1341-
sal_Int32 nPos = nCount;
1342-
1343-
// try to remove the last entries until new maximum size is reached
1344-
while( nCount > nVal )
1345-
{
1346-
SwOLEObj *const pObj = m_OleObjects[ --nPos ];
1347-
if ( pObj->UnloadObject() )
1348-
nCount--;
1349-
if ( !nPos )
1350-
break;
1351-
}
1352-
}
1353-
13541308
void SwOLELRUCache::Load()
13551309
{
13561310
Sequence< OUString > aNames( GetPropertyNames() );
@@ -1362,11 +1316,25 @@ void SwOLELRUCache::Load()
13621316

13631317
sal_Int32 nVal = 0;
13641318
*pValues >>= nVal;
1319+
13651320
if (nVal < m_nLRU_InitSize)
13661321
{
13671322
std::shared_ptr<SwOLELRUCache> xKeepAlive(g_pOLELRU_Cache); // prevent delete this
1368-
tryShrinkCacheTo(nVal);
1323+
// size of cache has been changed
1324+
sal_Int32 nCount = m_OleObjects.size();
1325+
sal_Int32 nPos = nCount;
1326+
1327+
// try to remove the last entries until new maximum size is reached
1328+
while( nCount > nVal )
1329+
{
1330+
SwOLEObj *const pObj = m_OleObjects[ --nPos ];
1331+
if ( pObj->UnloadObject() )
1332+
nCount--;
1333+
if ( !nPos )
1334+
break;
1335+
}
13691336
}
1337+
13701338
m_nLRU_InitSize = nVal;
13711339
}
13721340

@@ -1392,7 +1360,7 @@ void SwOLELRUCache::InsertObj( SwOLEObj& rObj )
13921360
if ( pObj->UnloadObject() )
13931361
nCount--;
13941362
}
1395-
m_OleObjects.insert(m_OleObjects.begin(), &rObj);
1363+
m_OleObjects.push_front(&rObj);
13961364
}
13971365

13981366
void SwOLELRUCache::RemoveObj( SwOLEObj& rObj )

0 commit comments

Comments
 (0)