zorba-coders team mailing list archive

[Chris Hillery <chillery+launchpad@xxxxxxxxx>]

Chris Hillery has proposed merging lp:~zorba-coders/zorba/xqxq-memory-smash into lp:zorba/xqxq-module.

Commit message:
Fix several memory errors having to do with freeing URIMappers / URLResolvers.

Requested reviews:
  Chris Hillery (ceejatec)
  Juan Zacarias (juan457)

For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/xqxq-memory-smash/+merge/130657
-- 
https://code.launchpad.net/~zorba-coders/zorba/xqxq-memory-smash/+merge/130657
Your team Zorba Coders is subscribed to branch lp:zorba/xqxq-module.

=== modified file 'src/xqxq.xq.src/xqxq.cpp'
--- src/xqxq.xq.src/xqxq.cpp	2012-10-19 01:22:47 +0000
+++ src/xqxq.xq.src/xqxq.cpp	2012-10-20 02:20:24 +0000
@@ -179,20 +179,39 @@
     String errDescription(aErrorMessage);
     throw USER_EXCEPTION(errQName, errDescription);
   }
-  
-  /*******************************************************************************************
-  *******************************************************************************************/
-  
+
+  /*******************************************************************************************
+  *******************************************************************************************/
+
+  QueryData::QueryData(XQuery_t aQuery, URIMapper *aMapper, URLResolver *aResolver)
+    : theQuery(aQuery),
+      theURIMapper(aMapper),
+      theURLResolver(aResolver)
+  {
+  }
+
+  QueryData::~QueryData()
+  {
+    theQuery->close();
+    delete theURIMapper;
+    delete theURLResolver;
+  }
+
+  /*******************************************************************************************
+  *******************************************************************************************/
+
   QueryMap::QueryMap()
   {
     QueryMap::queryMap = new QueryMap_t();
   }
 
   bool 
-    QueryMap::storeQuery(const String& aKeyName, XQuery_t aQuery)
+    QueryMap::storeQuery(const String& aKeyName, XQuery_t aQuery,
+                         URIMapper* aMapper, URLResolver* aResolver)
   {
+    QueryData_t lQueryData(new QueryData(aQuery, aMapper, aResolver));
     std::pair<QueryMap_t::iterator,bool> ret;
-    ret = queryMap->insert(std::pair<String, XQuery_t>(aKeyName, aQuery));
+    ret = queryMap->insert(std::pair<String, QueryData_t>(aKeyName, lQueryData));
     return ret.second;
   }
 
@@ -204,7 +223,7 @@
     if(lIter == queryMap->end())
       return NULL;
     
-    XQuery_t lQuery = lIter->second;
+    XQuery_t lQuery = lIter->second->getQuery();
 
     return lQuery;
   }
@@ -216,14 +235,27 @@
 
     if(lIter == queryMap->end())
       return false;
-    
-    lIter->second->close();
 
     queryMap->erase(lIter);
-
     return true;
   }
 
+  void
+    QueryMap::destroy() throw()
+  {
+    if(queryMap)
+    {
+      for (QueryMap_t::const_iterator lIter = queryMap->begin();
+           lIter != queryMap->end(); ++lIter)
+      {
+        deleteQuery(lIter->first);
+      }
+      queryMap->clear();
+      delete queryMap;
+    }
+    delete this;
+  }
+
   /*******************************************************************************************
   *******************************************************************************************/
   static void streamReleaser(std::istream* aStream)
@@ -344,7 +376,6 @@
   {
     DynamicContext* lDynCtx = const_cast<DynamicContext*>(aDctx);
     StaticContext_t lSctxChild = aSctx->createChildContext();
-    StaticContext_t lMapperSctx = aSctx->createChildContext();
    
     QueryMap* lQueryMap;
     if(!(lQueryMap = dynamic_cast<QueryMap*>(lDynCtx->getExternalFunctionParameter("xqxqQueryMap"))))
@@ -360,16 +391,16 @@
     XQuery_t lQuery;
     
     StaticContext_t ltempSctx = lZorba->createStaticContext();
-    XQXQURLResolver* lResolver = NULL;
-    XQXQURIMapper* lMapper = NULL;
+    std::auto_ptr<XQXQURLResolver> lResolver;
+    std::auto_ptr<XQXQURIMapper> lMapper;
 
     if ( aArgs.size() > 2 )
     {
       Item lMapperFunctionItem = getItemArgument(aArgs, 2);
       if (!lMapperFunctionItem.isNull())
       {
-        lMapper = new XQXQURIMapper(lMapperFunctionItem, lSctxChild);
-        ltempSctx->registerURIMapper(lMapper);
+        lMapper.reset(new XQXQURIMapper(lMapperFunctionItem, lSctxChild));
+        ltempSctx->registerURIMapper(lMapper.get());
       }
     }
 
@@ -378,8 +409,8 @@
       Item lResolverFunctionItem = getItemArgument(aArgs, 1);
       if (!lResolverFunctionItem.isNull())
       {
-        lResolver = new XQXQURLResolver(lResolverFunctionItem, lSctxChild);
-        ltempSctx->registerURLResolver(lResolver);
+        lResolver.reset(new XQXQURLResolver(lResolverFunctionItem, lSctxChild));
+        ltempSctx->registerURLResolver(lResolver.get());
       }
 
     }
@@ -390,6 +421,7 @@
     }
     catch (XQueryException& xe)
     {
+      lQuery = NULL;
       std::ostringstream err;
       err << "The query compiled using xqxq:prepare-main-module raised an error at"
           << " line " << xe.source_line() << " column " << xe.source_column() << ": " << xe.what();
@@ -399,6 +431,7 @@
     }
     catch (ZorbaException& e)
     {
+      lQuery = NULL;
       std::ostringstream err;
       err << "The query compiled using xqxq:prepare-main-module raised an error: "
           << e.what();
@@ -406,7 +439,7 @@
           e.diagnostic().qname().ns(), e.diagnostic().qname().localname());
       throw USER_EXCEPTION(errQName, err.str());
     }
-    
+
     uuid lUUID;
     uuid::create(&lUUID);
     
@@ -415,13 +448,8 @@
 
     String lStrUUID = lStream.str();
 
-	lQueryMap->storeQuery(lStrUUID, lQuery);
+    lQueryMap->storeQuery(lStrUUID, lQuery, lMapper.release(), lResolver.release());
     
-    if (lResolver)
-      delete lResolver;
-    if (lMapper)
-      delete lMapper;
-
     return ItemSequence_t(new SingletonItemSequence(XQXQModule::getItemFactory()->createAnyURI(lStrUUID)));
   }
 

=== modified file 'src/xqxq.xq.src/xqxq.h'
--- src/xqxq.xq.src/xqxq.h	2012-10-18 00:34:20 +0000
+++ src/xqxq.xq.src/xqxq.h	2012-10-20 02:20:24 +0000
@@ -49,35 +49,39 @@
 
   };
 
-
-  class QueryMap : public ExternalFunctionParameter{
-    private:
-      typedef std::map<String, XQuery_t> QueryMap_t;
+  /**
+   * @brief Bag class for objects associated with a prepared query
+   */
+  class QueryData : public SmartObject
+  {
+    private:
+      XQuery_t theQuery;
+      URIMapper* theURIMapper;
+      URLResolver* theURLResolver;
+
+    public:
+      QueryData(XQuery_t aQuery, URIMapper* aMapper, URLResolver* aResolver);
+      virtual ~QueryData();
+      XQuery_t getQuery() { return theQuery; }
+  };
+  typedef SmartPtr<QueryData> QueryData_t;
+
+  class QueryMap : public ExternalFunctionParameter
+  {
+    private:
+      typedef std::map<String, QueryData_t> QueryMap_t;
       QueryMap_t* queryMap;
 
     public:
       QueryMap();
       bool 
-        storeQuery(const String&, XQuery_t);
+        storeQuery(const String&, XQuery_t, URIMapper*, URLResolver*);
       XQuery_t
         getQuery(const String&);
       bool 
         deleteQuery(const String&);
       virtual void 
-        destroy() throw()
-      {
-        if(queryMap)
-        {
-          for (QueryMap_t::const_iterator lIter = queryMap->begin();
-               lIter != queryMap->end(); ++lIter)
-          {
-            lIter->second->close();
-          }
-          queryMap->clear();
-          delete queryMap;
-        }
-        delete this;
-      };
+        destroy() throw();
   };
 
   class XQXQFunction : public ContextualExternalFunction

=== added file 'test/ExpQueryResults/xqxq/url-schema-resolver3.xml.res'
--- test/ExpQueryResults/xqxq/url-schema-resolver3.xml.res	1970-01-01 00:00:00 +0000
+++ test/ExpQueryResults/xqxq/url-schema-resolver3.xml.res	2012-10-20 02:20:24 +0000
@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<test:test xmlns:test="http://test";><test:subtest>a</test:subtest><test:subtest2>a</test:subtest2></test:test>

=== added file 'test/Queries/xqxq/error-in-query.spec'
--- test/Queries/xqxq/error-in-query.spec	1970-01-01 00:00:00 +0000
+++ test/Queries/xqxq/error-in-query.spec	2012-10-20 02:20:24 +0000
@@ -0,0 +1,2 @@
+Error: http://www.w3.org/2005/xqt-errors:XQST0033
+

=== added file 'test/Queries/xqxq/error-in-query.xq'
--- test/Queries/xqxq/error-in-query.xq	1970-01-01 00:00:00 +0000
+++ test/Queries/xqxq/error-in-query.xq	2012-10-20 02:20:24 +0000
@@ -0,0 +1,21 @@
+import module namespace xqxq = 'http://www.zorba-xquery.com/modules/xqxq';
+
+declare namespace op = 'http://www.zorba-xquery.com/options/features';
+declare namespace f = 'http://www.zorba-xquery.com/features';
+declare option op:enable 'f:hof';
+
+declare function local:url-resolver($namespace as xs:string, $entity as xs:string) {
+switch($entity)
+case 'schema'
+  return switch($namespace)
+         case 'http://www.w3.org/XQueryTest' return doc('/tmp/atomic.xsd')
+         default return ()
+default return ()
+};
+
+variable $queryID := xqxq:prepare-main-module(
+  "declare namespace foo='http://test';
+   declare namespace foo='http://test';
+   1",
+   local:url-resolver#2, ());
+xqxq:evaluate($queryID)

=== added file 'test/Queries/xqxq/test.xml'
--- test/Queries/xqxq/test.xml	1970-01-01 00:00:00 +0000
+++ test/Queries/xqxq/test.xml	2012-10-20 02:20:24 +0000
@@ -0,0 +1,1 @@
+<test:test xmlns:test="http://test";><test:subtest>a</test:subtest><test:subtest2>a</test:subtest2></test:test>
\ No newline at end of file

=== added file 'test/Queries/xqxq/url-schema-resolver3.xq'
--- test/Queries/xqxq/url-schema-resolver3.xq	1970-01-01 00:00:00 +0000
+++ test/Queries/xqxq/url-schema-resolver3.xq	2012-10-20 02:20:24 +0000
@@ -0,0 +1,29 @@
+import module namespace xqxq = 'http://www.zorba-xquery.com/modules/xqxq';
+
+declare namespace resolver = 'http://www.zorba-xquery.com/modules/xqxq/url-resolver';
+
+declare namespace op = "http://www.zorba-xquery.com/options/features";;
+declare namespace f = "http://www.zorba-xquery.com/features";;
+declare option op:enable "f:hof";
+
+declare function resolver:url-resolver($namespace as xs:string, $entity as xs:string) {
+  if($namespace = 'http://test' and $entity = 'schema')
+  then 
+    doc('test.xsd')
+  else 
+    ()
+};
+
+variable $contextQueryID := xqxq:prepare-main-module(
+  "import schema namespace test = 'http://test';
+  declare variable $cwd as xs:anyURI external;
+  validate { doc(resolve-uri('test.xml', $cwd)) }",
+  resolver:url-resolver#2, ());
+xqxq:bind-variable($contextQueryID, fn:QName("", "cwd"), resolve-uri("."));
+variable $contextItem := xqxq:evaluate($contextQueryID);
+
+variable $queryID := xqxq:prepare-main-module(
+  "import schema namespace test = 'http://test'; //*:test",
+  resolver:url-resolver#2, ());
+xqxq:bind-context-item($queryID, $contextItem);
+xqxq:evaluate($queryID)