zorba-coders team mailing list archive

[Matthias Brantner <matthias.brantner@xxxxxxxxxx>]

Matthias Brantner has proposed merging lp:~zorba-coders/zorba/bug907872 into lp:zorba.

Requested reviews:
  Matthias Brantner (matthias-brantner)
  Markos Zaharioudakis (markos-za)
Related bugs:
  Bug #907872 in Zorba: "segfault when returning an input ItemSequence from an external function"
  https://bugs.launchpad.net/zorba/+bug/907872

For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738

fix for bug #907872 (segfault when returning an input ItemSequence from an external function). ExtFunctionCallIterator doesn't have the exclusive ownership over the ItemSequences such that their lifecycle can exceed the lifetime of the iterator itself.
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

=== modified file 'ChangeLog'
--- ChangeLog	2011-12-22 14:14:53 +0000
+++ ChangeLog	2011-12-22 19:16:24 +0000
@@ -10,7 +10,8 @@
   * Added index management function to the C++ api's StaticCollectionManager.
   * Fixed bug #905041 (allow for the default element and function namespaces to be
     set multiple times via the c++ api).
-  * Fixed bug #905050 (setting and getting the context item type via the c++ api)
+  * Fixed bug #907872 (segfault when returning an input ItemSequence from an external function).
+  * Fixed bug #905050 (setting and getting the context item type via the c++ api).
   * Added createDayTimeDuration, createYearMonthDuration, createDocumentNode, createCommentNode, createPiNode to api's ItemFactory.
 
 version 2.1

=== modified file 'src/runtime/core/fncall_iterator.cpp'
--- src/runtime/core/fncall_iterator.cpp	2011-12-21 14:40:33 +0000
+++ src/runtime/core/fncall_iterator.cpp	2011-12-22 19:16:24 +0000
@@ -630,7 +630,7 @@
 
   for (csize i = 0; i < n; ++i)
   {
-    delete m_extArgs[i];
+    m_extArgs[i]->removeReference();
   }
 }
 
@@ -737,6 +737,8 @@
   for(ulong i = 0; i < n; ++i)
   {
     state->m_extArgs[i] = new ExtFuncArgItemSequence(theChildren[i], planState);
+    // the iterator does not have exlcusive ownership over the sequences
+    state->m_extArgs[i]->addReference();
   }
 }
 

=== modified file 'test/unit/CMakeLists.txt'
--- test/unit/CMakeLists.txt	2011-12-21 14:40:33 +0000
+++ test/unit/CMakeLists.txt	2011-12-22 19:16:24 +0000
@@ -28,8 +28,10 @@
   
 #belongs to test external_function.cpp
 CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/ext_mod.xq ${CMAKE_CURRENT_BINARY_DIR}/ext_mod.xq)
+CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/ext_mod2.xq ${CMAKE_CURRENT_BINARY_DIR}/ext_mod2.xq)
 CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/ext_main.xq ${CMAKE_CURRENT_BINARY_DIR}/ext_main.xq)
 CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/ext_main2.xq ${CMAKE_CURRENT_BINARY_DIR}/ext_main2.xq)
+CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/ext_main3.xq ${CMAKE_CURRENT_BINARY_DIR}/ext_main3.xq)
 
 #belongs to test no_folding.cpp
 CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/fold_mod1.xq ${CMAKE_CURRENT_BINARY_DIR}/fold_mod1.xq)

=== added file 'test/unit/ext_main3.xq'
--- test/unit/ext_main3.xq	1970-01-01 00:00:00 +0000
+++ test/unit/ext_main3.xq	2011-12-22 19:16:24 +0000
@@ -0,0 +1,21 @@
+(:
+ : Copyright 2006-2009 The FLWOR Foundation.
+ :
+ : Licensed under the Apache License, Version 2.0 (the "License");
+ : you may not use this file except in compliance with the License.
+ : You may obtain a copy of the License at
+ :
+ : http://www.apache.org/licenses/LICENSE-2.0
+ :
+ : Unless required by applicable law or agreed to in writing, software
+ : distributed under the License is distributed on an "AS IS" BASIS,
+ : WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ : See the License for the specific language governing permissions and
+ : limitations under the License.
+:)
+
+
+import module namespace ext = "http://www.zorba-xquery.com/m"; at "file:///${CMAKE_CURRENT_BINARY_DIR}/ext_mod2.xq";
+
+
+ext:bar4(for $i in 1 to 10 return $i)

=== added file 'test/unit/ext_mod2.xq'
--- test/unit/ext_mod2.xq	1970-01-01 00:00:00 +0000
+++ test/unit/ext_mod2.xq	2011-12-22 19:16:24 +0000
@@ -0,0 +1,19 @@
+(:
+ : Copyright 2006-2009 The FLWOR Foundation.
+ :
+ : Licensed under the Apache License, Version 2.0 (the "License");
+ : you may not use this file except in compliance with the License.
+ : You may obtain a copy of the License at
+ :
+ : http://www.apache.org/licenses/LICENSE-2.0
+ :
+ : Unless required by applicable law or agreed to in writing, software
+ : distributed under the License is distributed on an "AS IS" BASIS,
+ : WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ : See the License for the specific language governing permissions and
+ : limitations under the License.
+:)
+
+module namespace ext = "http://www.zorba-xquery.com/m";;
+
+declare function ext:bar4($s as item()*) as item()* external;

=== modified file 'test/unit/external_function.cpp'
--- test/unit/external_function.cpp	2011-10-10 19:29:05 +0000
+++ test/unit/external_function.cpp	2011-12-22 19:16:24 +0000
@@ -125,6 +125,18 @@
   }
 };
 
+class MySimpleExternalFunction4 : public NonContextualExternalFunction
+{
+public:
+  String getURI() const { return "http://www.zorba-xquery.com/m";; }
+
+  String getLocalName() const { return "bar4"; }
+
+  ItemSequence_t evaluate(const ExternalFunction::Arguments_t& args) const
+  {
+    return args[0];
+  }
+};
 
 class MyExternalModule : public ExternalModule
 {
@@ -132,6 +144,7 @@
   MySimpleExternalFunction           bar;
   MySimpleExternalFunction2          bar2;
   MySimpleExternalFunction3          bar3;
+  MySimpleExternalFunction4          bar4;
 
 public:
   String getURI() const { return "http://www.zorba-xquery.com/m";; }
@@ -140,8 +153,10 @@
   {
     if (aLocalname == "bar")
         return const_cast<MySimpleExternalFunction*>(&bar);
-    if (aLocalname == "bar3")
+    else if (aLocalname == "bar3")
         return const_cast<MySimpleExternalFunction3*>(&bar3);
+    else if (aLocalname == "bar4")
+        return const_cast<MySimpleExternalFunction4*>(&bar4);
     else
         return const_cast<MySimpleExternalFunction2*>(&bar2);
   }
@@ -327,6 +342,42 @@
   return true;
 }
 
+bool
+external_function_test_4(Zorba* aZorba)
+{
+  try 
+  {
+    std::ifstream lIn("ext_main3.xq");
+    assert(lIn.good());
+    std::ostringstream lOut;
+    MyExternalModule lMod;
+
+    StaticContext_t lSctx = aZorba->createStaticContext();
+    lSctx->registerModule(&lMod);
+
+    {
+      XQuery_t lQuery = aZorba->compileQuery(lIn, lSctx);
+
+      std::cout << lQuery << std::endl;
+    }
+  } 
+  catch (XQueryException& qe) 
+  {
+    std::cerr << qe << std::endl;
+    return false;
+  }
+  catch (ZorbaException& e)
+  {
+    std::cerr << e << std::endl;
+    return false;
+  }
+  catch (...)
+  {
+    return false;
+  }
+  return true;
+}
+
 
 int
 external_function(int argc, char* argv[]) 
@@ -352,6 +403,13 @@
     return 3;
   }
 
+  std::cout << "executing external_function_test_4" << std::endl;
+  if (!external_function_test_4(lZorba))
+  {
+    return 4;
+  }
+
+
   lZorba->shutdown();
   zorba::StoreManager::shutdownStore(lStore);
   return 0;