[ 
https://issues.apache.org/jira/browse/AMQCPP-466?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13585487#comment-13585487
 ] 

Aaron Riekenberg commented on AMQCPP-466:
-----------------------------------------

Thanks.  I built your latest 3.6-SNAPSHOT in your directory and couldn't 
reproduce this problem with it.

I wonder if there are more changes coming in this area?  Couldn't help notice 
the TODO in 
[ActiveMQConnection::close|http://svn.apache.org/viewvc/activemq/activemq-cpp/trunk/activemq-cpp/src/main/activemq/core/ActiveMQConnection.cpp?view=markup]:

{code}
706             // As TemporaryQueue and TemporaryTopic instances are bound to 
a connection
707             // we should just delete them after the connection is closed to 
free up memory
708             Pointer<Iterator<Pointer<ActiveMQTempDestination> > > 
iterator(this->config->activeTempDestinations.values().iterator());
709     
710             try {
711                 while (iterator->hasNext()) {
712                     Pointer<ActiveMQTempDestination> dest = 
iterator->next();
713                     dest->close();
714                 }
715             } catch (cms::CMSException& error) {
716                 if (!hasException) {
717     //                ex = Exception(new CMSException(error));  TODO
718                     ex = Exception();
719                     ex.setMark(__FILE__, __LINE__);
720                     hasException = true;
721                 }
722             }
{code}
                
> Segmentation Fault in Temporary Queue consumer (incorrect Exception 
> construction)
> ---------------------------------------------------------------------------------
>
>                 Key: AMQCPP-466
>                 URL: https://issues.apache.org/jira/browse/AMQCPP-466
>             Project: ActiveMQ C++ Client
>          Issue Type: Bug
>          Components: CMS Impl
>    Affects Versions: 3.5.0
>         Environment: Ubuntu 12.10 x86_64, ActiveMQ 5.8.0
>            Reporter: Aaron Riekenberg
>            Assignee: Timothy Bish
>         Attachments: test.cpp
>
>
> I have a program that creates a TemporaryQueue consumer in CMS.  It listens 
> for exceptions from CMS and tries to close and recreate the connection when 
> an exception happens.  
> I'm finding the program crashes sometimes when closing the connection after 
> an exception.  I can't recreate this behavior with a consumer on a normal 
> non-temporary queue or topic.
> I've extracted the issue into a small test program (test.cpp) that I'm 
> attaching to this issue.
> Steps to reproduce:
> 1. Run activemq (activemq start)
> 2. Run attached test program
> 3. Stop activemq (activemq stop)
> 4. Restart activemq (activemq start)
> 5. Repeat steps 3 and 4.  Eventually the test program will crash with a 
> segmentation fault just after activemq is stopped and an exception is 
> detected.
> If I run the test program in valgrind, I see this output:
> {noformat:title=Test Program Valgrind Output}
> ==4055== Invalid read of size 8
> ==4055==    at 0x58E64D5: decaf::lang::Pointer<std::exception const, 
> decaf::util::concurrent::atomic::AtomicRefCounter>::onDeleteFunc(std::exception
>  const*) (in /home/aaron/amqcpp350/lib/libactivemq-cpp.so.15.0.0)
> ==4055==    by 0x58E546C: decaf::lang::Exception::~Exception() (Pointer.h:148)
> ==4055==    by 0x58E5538: decaf::lang::Exception::~Exception() 
> (Exception.cpp:107)
> ==4055==    by 0x588BBA4: cms::CMSException::~CMSException() (auto_ptr.h:170)
> ==4055==    by 0x5F090F8: ??? (in 
> /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.17)
> ==4055==    by 0x4034E7: SimpleAsyncConsumer::cleanup() (test.cpp:211)
> ==4055==    by 0x402D05: SimpleAsyncConsumer::close() (test.cpp:94)
> ==4055==    by 0x401EB6: main (test.cpp:278)
> ==4055==  Address 0x8427600 is 128 bytes inside a block of size 144 free'd
> ==4055==    at 0x4C2A739: free (in 
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==4055==    by 0x56747E8: activemq::core::ActiveMQConnection::close() 
> (ActiveMQConnection.cpp:714)
> ==4055==    by 0x4033F1: SimpleAsyncConsumer::cleanup() (test.cpp:210)
> ==4055==    by 0x402D05: SimpleAsyncConsumer::close() (test.cpp:94)
> ==4055==    by 0x401EB6: main (test.cpp:278)
> ==4055== 
> ==4055== Invalid write of size 8
> ==4055==    at 0x5F0817B: std::exception::~exception() (in 
> /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.17)
> ==4055==    by 0x5F081C8: std::exception::~exception() (in 
> /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.17)
> ==4055==    by 0x58E546C: decaf::lang::Exception::~Exception() (Pointer.h:148)
> ==4055==    by 0x58E5538: decaf::lang::Exception::~Exception() 
> (Exception.cpp:107)
> ==4055==    by 0x588BBA4: cms::CMSException::~CMSException() (auto_ptr.h:170)
> ==4055==    by 0x5F090F8: ??? (in 
> /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.17)
> ==4055==    by 0x4034E7: SimpleAsyncConsumer::cleanup() (test.cpp:211)
> ==4055==    by 0x402D05: SimpleAsyncConsumer::close() (test.cpp:94)
> ==4055==    by 0x401EB6: main (test.cpp:278)
> ==4055==  Address 0x8427600 is 128 bytes inside a block of size 144 free'd
> ==4055==    at 0x4C2A739: free (in 
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==4055==    by 0x56747E8: activemq::core::ActiveMQConnection::close() 
> (ActiveMQConnection.cpp:714)
> ==4055==    by 0x4033F1: SimpleAsyncConsumer::cleanup() (test.cpp:210)
> ==4055==    by 0x402D05: SimpleAsyncConsumer::close() (test.cpp:94)
> ==4055==    by 0x401EB6: main (test.cpp:278)
> ==4055== 
> ==4055== Invalid free() / delete / delete[] / realloc()
> ==4055==    at 0x4C2A44B: operator delete(void*) (in 
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==4055==    by 0x58E546C: decaf::lang::Exception::~Exception() (Pointer.h:148)
> ==4055==    by 0x58E5538: decaf::lang::Exception::~Exception() 
> (Exception.cpp:107)
> ==4055==    by 0x588BBA4: cms::CMSException::~CMSException() (auto_ptr.h:170)
> ==4055==    by 0x5F090F8: ??? (in 
> /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.17)
> ==4055==    by 0x4034E7: SimpleAsyncConsumer::cleanup() (test.cpp:211)
> ==4055==    by 0x402D05: SimpleAsyncConsumer::close() (test.cpp:94)
> ==4055==    by 0x401EB6: main (test.cpp:278)
> ==4055==  Address 0x8427600 is 128 bytes inside a block of size 144 free'd
> ==4055==    at 0x4C2A739: free (in 
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==4055==    by 0x56747E8: activemq::core::ActiveMQConnection::close() 
> (ActiveMQConnection.cpp:714)
> ==4055==    by 0x4033F1: SimpleAsyncConsumer::cleanup() (test.cpp:210)
> ==4055==    by 0x402D05: SimpleAsyncConsumer::close() (test.cpp:94)
> ==4055==    by 0x401EB6: main (test.cpp:278)
> {noformat}
> I believe the bug is ActiveMQConnection.cpp is doing this around line 714:
> {code:title=ActiveMQConnection.cpp line 716}
>  714         } catch (std::exception& stdex) {
>  715             if (!hasException) {
>  716                 ex = Exception(&stdex);
>  717                 ex.setMark(__FILE__, __LINE__);
>  718                 hasException = true;
>  719             }
>  720         }
> {code}
> The problem is line 716.  The pointer passed to the Exception constructor 
> transfers ownership to the Exception instance, meaning ~Exception will delete 
> &stdex.  But we don't own &stdex here.  stdex will automatically be destroyed 
> when we leave the catch block.
> So I think this code needs to clone the stdex instance here somehow before 
> passing it to Exception().  I think the reason this only happens with a 
> TemporaryQueue consumer is the code around line 716 is trying to clean up 
> temporary destinations and is skipped for a normal queue/topic.
> Note line 716 isn't the only instance of this problem.  Also see lines 645 
> and lines 710 for other instances of incorrectly creating an Exception that 
> will crash when they are executed:
> {code:title=ActiveMQConnection.cpp line 645}
>  643             } catch (cms::CMSException& error) {
>  644                 if (!hasException) {
>  645                     ex = Exception(&error);
>  646                     ex.setMark(__FILE__, __LINE__);
>  647                     hasException = true;
>  648                 }
>  649             }
> {code}
> {code:title=ActiveMQConnection.cpp line 710}
>  708         } catch (cms::CMSException& error) {
>  709             if (!hasException) {
>  710                 ex = Exception(&error);
>  711                 ex.setMark(__FILE__, __LINE__);
>  712                 hasException = true;
>  713             }
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to