[Libreoffice-commits] core.git: Branch 'libreoffice-5-2' - unoxml/source

Michael Stahl mstahl at redhat.com
Tue Jun 7 17:51:16 UTC 2016


 unoxml/source/events/eventdispatcher.cxx |   34 ++++++++++++-------------------
 unoxml/source/events/eventdispatcher.hxx |    2 -
 2 files changed, 15 insertions(+), 21 deletions(-)

New commits:
commit 9c9cd0684b53856f63286e49d7a13896cfc47c91
Author: Michael Stahl <mstahl at redhat.com>
Date:   Tue Jun 7 14:57:43 2016 +0200

    unoxml: fix data race in CEventDispatcher::callListeners()
    
    JunitTest_unoxml_complex crashed with an assertion:
    
        error: attempt to increment a singular iterator.
    
    This is because the CEventDispatcher::dispatchEvent() attempts to copy the
    m_TargetListeners onto the stack, before dropping the mutex to call the,
    listeners, but the copy is unintentionally shallow and the ListenerMap
    that is the value of the outer type is not copied, so this member is
    accessed in callListeners() without MutexGuard.
    
    Fix this by replacing ListenerMap* with ListenerMap as the value,
    which also allows getting rid of explicit delete calls.
    
    15 0x00002b7793e5ace8 in abort () at /lib64/libc.so.6
    16 0x00002b77948c1565 in __gnu_debug::_Error_formatter::_M_error() const () at /lib64/libstdc++.so.6
    17 0x00002b77b802a7d8 in __gnu_debug::_Safe_iterator<std::_Rb_tree_const_iterator<std::pair<_xmlNode* const, com::sun::star::uno::Reference<com::sun::star::xml::dom::events::XEventListener> > >, std::__debug::multimap<_xmlNode*, com::sun::star::uno::Reference<com::sun::star::xml::dom::events::XEventListener>, std::less<_xmlNode*>, std::allocator<std::pair<_xmlNode* const, com::sun::star::uno::Reference<com::sun::star::xml::dom::events::XEventListener> > > > >::operator++() (this=0x2b77b7d99c90) at /usr/include/c++/4.8.2/debug/safe_iterator.h:291
    18 0x00002b77b80275ae in DOM::events::CEventDispatcher::callListeners(std::__debug::map<rtl::OUString, std::__debug::multimap<_xmlNode*, com::sun::star::uno::Reference<com::sun::star::xml::dom::events::XEventListener>, std::less<_xmlNode*>, std::allocator<std::pair<_xmlNode* const, com::sun::star::uno::Reference<com::sun::star::xml::dom::events::XEventListener> > > >*, std::less<rtl::OUString>, std::allocator<std::pair<rtl::OUString const, std::__debug::multimap<_xmlNode*, com::sun::star::uno::Reference<com::sun::star::xml::dom::events::XEventListener>, std::less<_xmlNode*>, std::allocator<std::pair<_xmlNode* const, com::sun::star::uno::Reference<com::sun::star::xml::dom::events::XEventListener> > > >*> > > const&, _xmlNode*, rtl::OUString const&, com::sun::star::uno::Reference<com::sun::star::xml::dom::events::XEvent> const&) (rTMap=std::__debug::map with 1 elements, pNode=0x21a9c10, aType=..., xEvent=...) at /unoxml/source/events/eventdispatcher.cxx:103
            pMap = 0x2203b10
            iter = {first = , second = {<com::sun::star::uno::BaseReference> = {_pInterface = }, <No data fields>}}
            ibound = {first = , second = {<com::sun::star::uno::BaseReference> = {_pInterface = }, <No data fields>}}
            tIter = {first = {pData = }, second = }
    19 0x00002b77b80284e4 in DOM::events::CEventDispatcher::dispatchEvent(DOM::CDocument&, osl::Mutex&, _xmlNode*, com::sun::star::uno::Reference<com::sun::star::xml::dom::XNode> const&, com::sun::star::uno::Reference<com::sun::star::xml::dom::events::XEvent> const&) const (this=0x21fbee0, rDocument=..., rMutex=..., pNode=0x2200890, xNode=..., i_xEvent=...) at /unoxml/source/events/eventdispatcher.cxx:242
            captureListeners = std::__debug::map with 0 elements
            targetListeners = std::__debug::map with 1 elements = {[{pData = 0x2202a30}] = 0x2203b10}
    
    Change-Id: I66fb7a461df0f8066383365850536f7b0394306d
    (cherry picked from commit 93f3bd545b65a4df1ed04118f696a85a0e50c423)
    Reviewed-on: https://gerrit.libreoffice.org/26017
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>

diff --git a/unoxml/source/events/eventdispatcher.cxx b/unoxml/source/events/eventdispatcher.cxx
index 7fd7a48..0dab5b9 100644
--- a/unoxml/source/events/eventdispatcher.cxx
+++ b/unoxml/source/events/eventdispatcher.cxx
@@ -41,16 +41,16 @@ namespace DOM { namespace events {
 
         // get the multimap for the specified type
         ListenerMap *pMap = nullptr;
-        TypeListenerMap::const_iterator tIter = pTMap->find(aType);
+        auto tIter = pTMap->find(aType);
         if (tIter == pTMap->end()) {
             // the map has to be created
-            pMap = new ListenerMap();
-            pTMap->insert(TypeListenerMap::value_type(aType, pMap));
+            auto const pair = pTMap->insert(TypeListenerMap::value_type(aType, ListenerMap()));
+            pMap = & pair.first->second;
         } else {
-            pMap = tIter->second;
+            pMap = & tIter->second;
         }
-        if (pMap !=nullptr)
-            pMap->insert(ListenerMap::value_type(pNode, aListener));
+        assert(pMap != nullptr);
+        pMap->insert(ListenerMap::value_type(pNode, aListener));
     }
 
     void CEventDispatcher::removeListener(xmlNodePtr pNode, const OUString& aType, const Reference<XEventListener>& aListener, bool bCapture)
@@ -59,19 +59,19 @@ namespace DOM { namespace events {
             ? (& m_CaptureListeners) : (& m_TargetListeners);
 
         // get the multimap for the specified type
-        TypeListenerMap::const_iterator tIter = pTMap->find(aType);
+        auto tIter = pTMap->find(aType);
         if (tIter != pTMap->end()) {
-            ListenerMap *pMap = tIter->second;
+            ListenerMap & rMap = tIter->second;
             // find listeners of specified type for specified node
-            ListenerMap::iterator iter = pMap->find(pNode);
-            while (iter != pMap->end() && iter->first == pNode)
+            ListenerMap::iterator iter = rMap.find(pNode);
+            while (iter != rMap.end() && iter->first == pNode)
             {
                 // erase all references to specified listener
                 if ((iter->second).is() && iter->second == aListener)
                 {
                     ListenerMap::iterator tmp_iter = iter;
                     ++iter;
-                    pMap->erase(tmp_iter);
+                    rMap.erase(tmp_iter);
                 }
                 else
                     ++iter;
@@ -81,12 +81,6 @@ namespace DOM { namespace events {
 
     CEventDispatcher::~CEventDispatcher()
     {
-        // delete the multimaps for the various types
-        for (TypeListenerMap::iterator aI = m_CaptureListeners.begin(); aI != m_CaptureListeners.end(); ++aI)
-            delete aI->second;
-
-        for (TypeListenerMap::iterator aI = m_TargetListeners.begin(); aI != m_TargetListeners.end(); ++aI)
-            delete aI->second;
     }
 
     void CEventDispatcher::callListeners(
@@ -97,9 +91,9 @@ namespace DOM { namespace events {
         // get the multimap for the specified type
         TypeListenerMap::const_iterator tIter = rTMap.find(aType);
         if (tIter != rTMap.end()) {
-            ListenerMap *pMap = tIter->second;
-            ListenerMap::const_iterator iter = pMap->lower_bound(pNode);
-            ListenerMap::const_iterator ibound = pMap->upper_bound(pNode);
+            ListenerMap const& rMap = tIter->second;
+            auto iter = rMap.lower_bound(pNode);
+            auto const ibound = rMap.upper_bound(pNode);
             for( ; iter != ibound; ++iter )
             {
                 if((iter->second).is())
diff --git a/unoxml/source/events/eventdispatcher.hxx b/unoxml/source/events/eventdispatcher.hxx
index d8703ac..d992ff3 100644
--- a/unoxml/source/events/eventdispatcher.hxx
+++ b/unoxml/source/events/eventdispatcher.hxx
@@ -42,7 +42,7 @@ class CDocument;
 namespace events {
 
 typedef std::multimap< xmlNodePtr, css::uno::Reference< css::xml::dom::events::XEventListener> > ListenerMap;
-typedef std::map< OUString, ListenerMap*> TypeListenerMap;
+typedef std::map<OUString, ListenerMap> TypeListenerMap;
 
 class CEventDispatcher
 {


More information about the Libreoffice-commits mailing list