Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba
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
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
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
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
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
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
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
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
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
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
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
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
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
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