xmlfix3: #i113682#: unoxml: locking in XPath:

 use CDocument mutex in results, and during evaluation.
 new CXPathAPI member mutex.
 also, fix lcl_collectNamespaces in case parent nodes declare same prefix.

unoxml/source/dom/document.hxx

@@ -113,6 +113,9 @@ namespace DOM
 
         virtual ~CDocument();
 
+        // needed by CXPathAPI
+        ::osl::Mutex & GetMutex() { return m_Mutex; }
+
         events::CEventDispatcher & GetEventDispatcher();
         ::rtl::Reference< CElement > GetDocumentElement();
 

unoxml/source/xpath/nodelist.cxx

@@ -25,15 +25,18 @@
  *
  ************************************************************************/
 
-#include "nodelist.hxx"
+#include <nodelist.hxx>
+
 #include "../dom/document.hxx"
 
 namespace XPath
 {
     CNodeList::CNodeList(
                 ::rtl::Reference<DOM::CDocument> const& pDocument,
+                ::osl::Mutex & rMutex,
                 boost::shared_ptr<xmlXPathObject> const& rxpathObj)
         : m_pDocument(pDocument)
+        , m_rMutex(rMutex)
         , m_pNodeSet(0)
     {
         if (rxpathObj != NULL && rxpathObj->type == XPATH_NODESET)
@@ -48,6 +51,8 @@ namespace XPath
     */
     sal_Int32 SAL_CALL CNodeList::getLength() throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         sal_Int32 value = 0;
         if (m_pNodeSet != NULL)
             value = xmlXPathNodeSetGetLength(m_pNodeSet);
@@ -60,6 +65,8 @@ namespace XPath
     Reference< XNode > SAL_CALL CNodeList::item(sal_Int32 index)
         throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         if (0 == m_pNodeSet) {
             return 0;
         }

unoxml/source/xpath/nodelist.hxx

@@ -59,14 +59,16 @@ namespace XPath
     class CNodeList : public cppu::WeakImplHelper1< XNodeList >
     {
     private:
-        /// #i115995# retain context node to keep document alive
+        /// #i115995# keep document alive
         ::rtl::Reference< DOM::CDocument > const m_pDocument;
+        ::osl::Mutex & m_rMutex;
         boost::shared_ptr<xmlXPathObject> m_pXPathObj;
         xmlNodeSetPtr m_pNodeSet;
 
     public:
         CNodeList(
                 ::rtl::Reference<DOM::CDocument> const& pDocument,
+                ::osl::Mutex & rMutex,
                 boost::shared_ptr<xmlXPathObject> const& rxpathObj);
         /**
         The number of nodes in the list.

unoxml/source/xpath/xpathapi.cxx

@@ -25,11 +25,12 @@
  *
  ************************************************************************/
 
-#include "xpathapi.hxx"
+#include <xpathapi.hxx>
 
 #include <stdarg.h>
 #include <string.h>
 
+#include <libxml/tree.h>
 #include <libxml/xmlerror.h>
 #include <libxml/xpath.h>
 #include <libxml/xpathInternals.h>
@@ -38,6 +39,7 @@
 
 #include <nodelist.hxx>
 #include <xpathobject.hxx>
+
 #include "../dom/node.hxx"
 #include "../dom/document.hxx"
 
@@ -110,6 +112,8 @@ namespace XPath
             const OUString& aURI)
         throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_Mutex);
+
         m_nsmap.insert(nsmap_t::value_type(aPrefix, aURI));
     }
 
@@ -118,13 +122,16 @@ namespace XPath
             const OUString& aURI)
         throw (RuntimeException)
     {
-        if ((m_nsmap.find(aPrefix))->second.equals(aURI))
+        ::osl::MutexGuard const g(m_Mutex);
+
+        if ((m_nsmap.find(aPrefix))->second.equals(aURI)) {
             m_nsmap.erase(aPrefix);
+        }
     }
 
     // register all namespaces stored in the namespace list for this object
     // with the current xpath evaluation context
-    static void _registerNamespaces(
+    static void lcl_registerNamespaces(
             xmlXPathContextPtr ctx,
             const nsmap_t& nsmap)
     {
@@ -142,13 +149,15 @@ namespace XPath
         }
     }
 
-    // get all ns decls on a node (and parent nodes, if any) and register them
+    // get all ns decls on a node (and parent nodes, if any)
     static void lcl_collectNamespaces(
-            CXPathAPI *const pAPI,
-            Reference< XNode > const& xNamespaceNode)
+            nsmap_t & rNamespaces, Reference< XNode > const& xNamespaceNode)
     {
-        // get namespace decls from node...
         DOM::CNode *const pCNode(DOM::CNode::GetImplementation(xNamespaceNode));
+        if (pCNode) { throw RuntimeException(); }
+
+        ::osl::MutexGuard const g(pCNode->GetOwnerDocument().GetMutex());
+
         xmlNodePtr pNode = pCNode->GetNodePtr();
         while (pNode != 0) {
             xmlNsPtr curDef = pNode->nsDef;
@@ -157,16 +166,32 @@ namespace XPath
                 OUString aURI((sal_Char*)xHref, strlen((char*)xHref), RTL_TEXTENCODING_UTF8);
                 const xmlChar* xPre = curDef->prefix;
                 OUString aPrefix((sal_Char*)xPre, strlen((char*)xPre), RTL_TEXTENCODING_UTF8);
-                pAPI->registerNS(aPrefix, aURI);
+                // we could already have this prefix from a child node
+                if (rNamespaces.find(aPrefix) == rNamespaces.end())
+                {
+                    rNamespaces.insert(::std::make_pair(aPrefix, aURI));
+                }
                 curDef = curDef->next;
             }
             pNode = pNode->parent;
         }
     }
 
+    static void lcl_collectRegisterNamespaces(
+            CXPathAPI & rAPI, Reference< XNode > const& xNamespaceNode)
+    {
+        nsmap_t namespaces;
+        lcl_collectNamespaces(namespaces, xNamespaceNode);
+        for (nsmap_t::const_iterator iter = namespaces.begin();
+                iter != namespaces.end(); ++iter)
+        {
+            rAPI.registerNS(iter->first, iter->second);
+        }
+    }
+
     // register function and variable lookup functions with the current
     // xpath evaluation context
-    static void _registerExtensions(
+    static void lcl_registerExtensions(
             xmlXPathContextPtr ctx,
             const extensions_t& extensions)
     {
@@ -215,7 +240,7 @@ namespace XPath
             const Reference< XNode >&  namespaceNode)
         throw (RuntimeException, XPathException)
     {
-        lcl_collectNamespaces(this, namespaceNode);
+        lcl_collectRegisterNamespaces(*this, namespaceNode);
         return selectNodeList(contextNode, expr);
     }
 
@@ -242,7 +267,7 @@ namespace XPath
             const Reference< XNode >&  namespaceNode )
         throw (RuntimeException, XPathException)
     {
-        lcl_collectNamespaces(this, namespaceNode);
+        lcl_collectRegisterNamespaces(*this, namespaceNode);
         return selectSingleNode(contextNode, expr);
     }
 
@@ -313,16 +338,35 @@ namespace XPath
      * the context Node
      */
     Reference< XXPathObject > SAL_CALL CXPathAPI::eval(
-            const Reference< XNode >& contextNode,
+            Reference< XNode > const& xContextNode,
             const OUString& expr)
         throw (RuntimeException, XPathException)
     {
+        if (!xContextNode.is()) { throw RuntimeException(); }
+
+        nsmap_t nsmap;
+        extensions_t extensions;
+
+        {
+            ::osl::MutexGuard const g(m_Mutex);
+            nsmap = m_nsmap;
+            extensions = m_extensions;
+        }
+
         xmlXPathContextPtr xpathCtx;
         xmlXPathObjectPtr xpathObj;
 
         // get the node and document
-        DOM::CNode *const pCNode = DOM::CNode::GetImplementation(contextNode);
+        ::rtl::Reference<DOM::CDocument> const pCDoc(
+                dynamic_cast<DOM::CDocument*>( DOM::CNode::GetImplementation(
+                        xContextNode->getOwnerDocument())));
+        if (!pCDoc.is()) { throw RuntimeException(); }
+
+        DOM::CNode *const pCNode = DOM::CNode::GetImplementation(xContextNode);
         if (!pCNode) { throw RuntimeException(); }
+
+        ::osl::MutexGuard const g(pCDoc->GetMutex()); // lock the document!
+
         xmlNodePtr const pNode = pCNode->GetNodePtr();
         if (!pNode) { throw RuntimeException(); }
         xmlDocPtr pDoc = pNode->doc;
@@ -349,8 +393,8 @@ namespace XPath
         xmlSetGenericErrorFunc(NULL, generic_error_func);
 
         // register namespaces and extension
-        _registerNamespaces(xpathCtx, m_nsmap);
-        _registerExtensions(xpathCtx, m_extensions);
+        lcl_registerNamespaces(xpathCtx, nsmap);
+        lcl_registerExtensions(xpathCtx, extensions);
 
         /* run the query */
         OString o1 = OUStringToOString(expr, RTL_TEXTENCODING_UTF8);
@@ -361,10 +405,8 @@ namespace XPath
             throw XPathException();
         }
         xmlXPathFreeContext(xpathCtx);
-        ::rtl::Reference<DOM::CDocument> const pCDoc(
-                & pCNode->GetOwnerDocument());
-        OSL_ASSERT(pCDoc.is());
-        Reference<XXPathObject> const xObj(new CXPathObject(pCDoc, xpathObj));
+        Reference<XXPathObject> const xObj(
+                new CXPathObject(pCDoc, pCDoc->GetMutex(), xpathObj));
         return xObj;
     }
 
@@ -377,7 +419,7 @@ namespace XPath
             const Reference< XNode >& namespaceNode)
         throw (RuntimeException, XPathException)
     {
-        lcl_collectNamespaces(this, namespaceNode);
+        lcl_collectRegisterNamespaces(*this, namespaceNode);
         return eval(contextNode, expr);
     }
 
@@ -390,9 +432,12 @@ namespace XPath
             const OUString& aName)
         throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_Mutex);
+
         // get extension from service manager
-        Reference< XXPathExtension > aExtension(m_aFactory->createInstance(aName), UNO_QUERY_THROW);
-        m_extensions.push_back( aExtension );
+        Reference< XXPathExtension > const xExtension(
+                m_aFactory->createInstance(aName), UNO_QUERY_THROW);
+        m_extensions.push_back(xExtension);
     }
 
     /**
@@ -400,13 +445,13 @@ namespace XPath
      * XPathAPI instance
      */
     void SAL_CALL CXPathAPI::registerExtensionInstance(
-            const Reference< XXPathExtension>& aExtension)
+            Reference< XXPathExtension> const& xExtension)
         throw (RuntimeException)
     {
-        if (aExtension.is()) {
-            m_extensions.push_back( aExtension );
-        } else {
+        if (!xExtension.is()) {
             throw RuntimeException();
         }
+        ::osl::MutexGuard const g(m_Mutex);
+        m_extensions.push_back( xExtension );
     }
 }

unoxml/source/xpath/xpathapi.hxx

@@ -25,14 +25,16 @@
  *
  ************************************************************************/
 
-#ifndef _XPATHAPI_HXX
-#define _XPATHAPI_HXX
+#ifndef XPATH_XPATHAPI_HXX
+#define XPATH_XPATHAPI_HXX
 
 #include <map>
 #include <vector>
 
 #include <sal/types.h>
+
 #include <cppuhelper/implbase2.hxx>
+
 #include <com/sun/star/uno/Reference.h>
 #include <com/sun/star/uno/Sequence.h>
 
@@ -50,7 +52,6 @@
 #include <com/sun/star/lang/XSingleServiceFactory.hpp>
 #include <com/sun/star/lang/XMultiServiceFactory.hpp>
 
-#include "libxml/tree.h"
 
 using ::rtl::OUString;
 using namespace com::sun::star::uno;
@@ -72,6 +73,7 @@ namespace XPath
     {
 
     private:
+        ::osl::Mutex m_Mutex;
         nsmap_t m_nsmap;
         const Reference< ::com::sun::star::lang::XMultiServiceFactory > m_aFactory;
         extensions_t m_extensions;

unoxml/source/xpath/xpathobject.cxx

@@ -66,8 +66,10 @@ namespace XPath
 
     CXPathObject::CXPathObject(
             ::rtl::Reference<DOM::CDocument> const& pDocument,
+            ::osl::Mutex & rMutex,
             xmlXPathObjectPtr const xpathObj)
         : m_pDocument(pDocument)
+        , m_rMutex(rMutex)
         , m_pXPathObj(xpathObj, xmlXPathFreeObject)
         , m_XPathObjectType(lcl_GetType(xpathObj))
     {
@@ -87,8 +89,10 @@ namespace XPath
     Reference< XNodeList > SAL_CALL
     CXPathObject::getNodeList() throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         Reference< XNodeList > const xRet(
-            new CNodeList(m_pDocument, m_pXPathObj));
+            new CNodeList(m_pDocument, m_rMutex, m_pXPathObj));
         return xRet;
     }
 
@@ -97,6 +101,8 @@ namespace XPath
      */
     sal_Bool SAL_CALL CXPathObject::getBoolean() throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         return (sal_Bool) xmlXPathCastToBoolean(m_pXPathObj.get());
     }
 
@@ -105,6 +111,8 @@ namespace XPath
     */
     sal_Int8 SAL_CALL CXPathObject::getByte() throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         return (sal_Int8) xmlXPathCastToNumber(m_pXPathObj.get());
     }
 
@@ -113,6 +121,8 @@ namespace XPath
     */
     sal_Int16 SAL_CALL CXPathObject::getShort() throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         return (sal_Int16) xmlXPathCastToNumber(m_pXPathObj.get());
     }
 
@@ -121,6 +131,8 @@ namespace XPath
     */
     sal_Int32 SAL_CALL CXPathObject::getLong() throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         return (sal_Int32) xmlXPathCastToNumber(m_pXPathObj.get());
     }
 
@@ -129,6 +141,8 @@ namespace XPath
     */
     sal_Int64 SAL_CALL CXPathObject::getHyper() throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         return (sal_Int64) xmlXPathCastToNumber(m_pXPathObj.get());
     }
 
@@ -137,6 +151,8 @@ namespace XPath
     */
     float SAL_CALL CXPathObject::getFloat() throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         return (float) xmlXPathCastToNumber(m_pXPathObj.get());
     }
 
@@ -145,6 +161,8 @@ namespace XPath
     */
     double SAL_CALL CXPathObject::getDouble() throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         return  xmlXPathCastToNumber(m_pXPathObj.get());
     }
 
@@ -153,6 +171,8 @@ namespace XPath
     */
     OUString SAL_CALL CXPathObject::getString() throw (RuntimeException)
     {
+        ::osl::MutexGuard const g(m_rMutex);
+
         const sal_Char* x1 = (sal_Char*) xmlXPathCastToString(m_pXPathObj.get());
         return OUString(x1, strlen(x1), RTL_TEXTENCODING_UTF8);
     }

unoxml/source/xpath/xpathobject.hxx

@@ -59,11 +59,13 @@ namespace XPath
     {
     private:
         ::rtl::Reference< DOM::CDocument > const m_pDocument;
+        ::osl::Mutex & m_rMutex;
         boost::shared_ptr<xmlXPathObject> const m_pXPathObj;
         XPathObjectType const m_XPathObjectType;
 
     public:
         CXPathObject( ::rtl::Reference<DOM::CDocument> const& pDocument,
+            ::osl::Mutex & rMutex,
             xmlXPathObjectPtr const xpathObj);
 
     /**