Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2012-01-03 Thread Markos Zaharioudakis
Review: Approve


-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2012-01-03 Thread Markos Zaharioudakis
The ExtFuncArgItemSequence cannot live longer than the ExtFunctionCallIterator 
who created it, because it is just a wrapper over a child of the 
ExtFunctionCallIterator, and if the ExtFunctionCallIterator is destroyed, its 
children are destroyed as well.

The real problem is that the ExtFuncArgItemSequence is assigned to an 
ItemSequence_t (the return value of the evaluate() method). Since 
ItemSequence_t is a smart pointer, it will destroy the ExtFuncArgItemSequence 
when itself is destroyed. Later, when the ExtFunctionCallIterator is closed, it 
will also try to destroy the ExtFuncArgItemSequence.

Your fix solves this problem, so I have approved it. 


 Did you also try the example (without my fix) and valgrind? I don't get a
 crash either but valgrind shows that there are serious problems:
 
 ==12383== Invalid read of size 4
 ==12383==at 0x448BDFF: zorba::SmartPtrzorba::ItemSequence::~SmartPtr()
 (smart_ptr.h:82)
 ==12383==by 0x4D25F43:
 zorba::ExtFunctionCallIteratorState::~ExtFunctionCallIteratorState()
 (fncall_iterator.cpp:625)
 ==12383==by 0x4D2FE87: zorba::StateTraitsImplzorba::ExtFunctionCallIterat
 orState::destroyState(zorba::PlanState, unsigned int) (plan_iterator.h:283)
 ==12383==by 0x4D2FE4D:
 zorba::NaryBaseIteratorzorba::ExtFunctionCallIterator,
 zorba::ExtFunctionCallIteratorState::closeImpl(zorba::PlanState)
 (narybase.h:164)
 ==12383==by 0x4D2F815:
 zorba::Batcherzorba::ExtFunctionCallIterator::close(zorba::PlanState)
 (plan_iterator.h:563)
 ==12383==by 0x4CAFC27: zorba::PlanWrapper::close() (plan_wrapper.cpp:179)
 ==12383==by 0x4457EFB: zorba::XQueryImpl::execute(std::ostream,
 Zorba_SerializerOptions const*) (xqueryimpl.cpp:1124)
 ==12383==by 0x4459B13: zorba::operator(std::ostream,
 zorba::SmartPtrzorba::XQuery const) (xqueryimpl.cpp:1580)
 ==12383==by 0x8068B83: external_function_test_4(zorba::Zorba*)
 (external_function.cpp:361)
 ==12383==by 0x8068E6B: external_function(int, char**)
 (external_function.cpp:407)
 ==12383==by 0x80613C4: main (UnitTests.cpp:270)
 ==12383==  Address 0x7e00e28 is 0 bytes inside a block of size 24 free'd
 ==12383==at 0x4025907: operator delete(void*) (vg_replace_malloc.c:387)
 ==12383==by 0x4D2F3F5:
 zorba::ExtFuncArgItemSequence::~ExtFuncArgItemSequence()
 (fncall_iterator.cpp:539)
 ==12383==by 0x4D25F16:
 zorba::ExtFunctionCallIteratorState::~ExtFunctionCallIteratorState()
 (fncall_iterator.cpp:633)
 ==12383==by 0x4D2FE87: zorba::StateTraitsImplzorba::ExtFunctionCallIterat
 orState::destroyState(zorba::PlanState, unsigned int) (plan_iterator.h:283)
 ==12383==by 0x4D2FE4D:
 zorba::NaryBaseIteratorzorba::ExtFunctionCallIterator,
 zorba::ExtFunctionCallIteratorState::closeImpl(zorba::PlanState)
 (narybase.h:164)
 ==12383==by 0x4D2F815:
 zorba::Batcherzorba::ExtFunctionCallIterator::close(zorba::PlanState)
 (plan_iterator.h:563)
 ==12383==by 0x4CAFC27: zorba::PlanWrapper::close() (plan_wrapper.cpp:179)
 ==12383==by 0x4457EFB: zorba::XQueryImpl::execute(std::ostream,
 Zorba_SerializerOptions const*) (xqueryimpl.cpp:1124)
 ==12383==by 0x4459B13: zorba::operator(std::ostream,
 zorba::SmartPtrzorba::XQuery const) (xqueryimpl.cpp:1580)
 ==12383==by 0x8068B83: external_function_test_4(zorba::Zorba*)
 (external_function.cpp:361)
 ==12383==by 0x8068E6B: external_function(int, char**)
 (external_function.cpp:407)
 ==12383==by 0x80613C4: main (UnitTests.cpp:270)
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2012-01-03 Thread Markos Zaharioudakis
The proposal to merge lp:~zorba-coders/zorba/bug907872 into lp:zorba has been 
updated.

Status: Needs review = Approved

For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2012-01-03 Thread Zorba Build Bot
Validation queue starting for merge proposal.
Log at: 
http://zorbatest.lambda.nu:8080/remotequeue/bug907872-2012-01-03T13-15-02.06Z/log.html
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2012-01-03 Thread Zorba Build Bot
Validation queue job bug907872-2012-01-03T13-15-02.06Z is finished. The final 
status was:

All tests succeeded!
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2012-01-03 Thread noreply
The proposal to merge lp:~zorba-coders/zorba/bug907872 into lp:zorba has been 
updated.

Status: Approved = Merged

For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Matthias Brantner
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 +
+++ ChangeLog	2011-12-22 19:16:24 +
@@ -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 +
+++ src/runtime/core/fncall_iterator.cpp	2011-12-22 19:16:24 +
@@ -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 +
+++ test/unit/CMakeLists.txt	2011-12-22 19:16:24 +
@@ -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 +
+++ test/unit/ext_main3.xq	2011-12-22 19:16:24 +
@@ -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 +
+++ test/unit/ext_mod2.xq	2011-12-22 19:16:24 +
@@ -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 +
+++ test/unit/external_function.cpp	2011-12-22 

Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Matthias Brantner
Review: Approve


-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Matthias Brantner
The proposal to merge lp:~zorba-coders/zorba/bug907872 into lp:zorba has been 
updated.

Status: Needs review = Approved

For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Zorba Build Bot
Validation queue starting for merge proposal.
Log at: 
http://zorbatest.lambda.nu:8080/remotequeue/bug907872-2011-12-22T19-18-00.125Z/log.html
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Zorba Build Bot
Validation queue job bug907872-2011-12-22T19-18-00.125Z is finished. The final 
status was:

All tests succeeded!
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Zorba Build Bot
Voting does not meet specified criteria. Required: Approve  1, Disapprove  1. 
Got: 1 Approve, 2 Pending.
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


[Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Zorba Build Bot
The proposal to merge lp:~zorba-coders/zorba/bug907872 into lp:zorba has been 
updated.

Status: Approved = Needs review

For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Markos Zaharioudakis
Matthias, I cannot see how the example you added illustrates the case where an 
ExtFuncArgItemSequence must live longer than the ExtFunctionCallIterator who 
created it. In fact, I tried the example without your modification in 
fncall_iterator.cpp and it worked fine. What am I missing?
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp