You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by "Timothy Bish (JIRA)" <ji...@apache.org> on 2013/02/24 22:04:12 UTC

[jira] [Resolved] (AMQCPP-466) Segmentation Fault in Temporary Queue consumer (incorrect Exception construction)

     [ https://issues.apache.org/jira/browse/AMQCPP-466?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Timothy Bish resolved AMQCPP-466.
---------------------------------

       Resolution: Fixed
    Fix Version/s: 3.6.0

Fixed on trunk
                
> 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
>             Fix For: 3.6.0
>
>         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