ActiveMQ C++ Client
  1. ActiveMQ C++ Client
  2. AMQCPP-527

ConcurrentStlMap - stl map find crash with empty map

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.8.1
    • Fix Version/s: 3.8.3, 3.9.0
    • Component/s: Decaf
    • Labels:
      None

      Description

      Recently experience a crash while an activemq work thread attempts to remove a temporary destination from a ConcurrentStlMap. This crash happens inside a std::map::find() when the map is currently empty.

      virtual V remove(const K& key) {
                  V result = V();
                  synchronized(&mutex) {
                      typename std::map<K, V, COMPARATOR>::iterator iter = valueMap.find(key);
                      if (iter == valueMap.end()) {
                          return result;
                      }
      [...]
      

      I look arround in forums and found that some people do experience similar problems with stl map.

      http://social.msdn.microsoft.com/Forums/en-US/9fdd11cd-ab7a-4173-8fcc-ca239b10fd20/weird-crash-in-c-mapsfind-when-compiled-in-release-mode

      http://www.velocityreviews.com/forums/t278300-std-map-find-throws-exception-when-map-is-empty.html

      so it appears that this error might be an stl issue, (perhaps platform related?). The workaround suggested is the check if the map is empty before performing a find.

      The stack trace i got

       	activemq-cpp.dll!std::_Tree<std::_Tmap_traits<decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,decaf::lang::PointerComparator<activemq::commands::ActiveMQDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,std::allocator<std::pair<decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> const ,decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> > >,0> >::_Lbound(const decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> & _Keyval={...})  Line 1264 + 0x8 bytes	C++
       	activemq-cpp.dll!std::_Tree<std::_Tmap_traits<decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,decaf::lang::PointerComparator<activemq::commands::ActiveMQDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,std::allocator<std::pair<decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> const ,decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> > >,0> >::lower_bound(const decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> & _Keyval={...})  Line 1004 + 0x10 bytes	C++
       	activemq-cpp.dll!std::_Tree<std::_Tmap_traits<decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,decaf::lang::PointerComparator<activemq::commands::ActiveMQDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,std::allocator<std::pair<decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> const ,decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> > >,0> >::find(const decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> & _Keyval={...})  Line 982	C++
      >	activemq-cpp.dll!decaf::util::concurrent::ConcurrentStlMap<decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter>,decaf::lang::PointerComparator<activemq::commands::ActiveMQDestination,decaf::util::concurrent::atomic::AtomicRefCounter> >::remove(const decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> & key={...})  Line 920	C++
       	activemq-cpp.dll!activemq::core::ActiveMQConnection::removeTempDestination(decaf::lang::Pointer<activemq::commands::ActiveMQTempDestination,decaf::util::concurrent::atomic::AtomicRefCounter> destination={...})  Line 1828 + 0x25 bytes	C++
       	activemq-cpp.dll!activemq::core::AdvisoryConsumer::processDestinationInfo(decaf::lang::Pointer<activemq::commands::DestinationInfo,decaf::util::concurrent::atomic::AtomicRefCounter> info={...})  Line 154	C++
       	activemq-cpp.dll!activemq::core::AdvisoryConsumer::dispatch(const decaf::lang::Pointer<activemq::commands::MessageDispatch,decaf::util::concurrent::atomic::AtomicRefCounter> & message={...})  Line 135	C++
       	activemq-cpp.dll!activemq::core::ActiveMQConnection::onCommand(decaf::lang::Pointer<activemq::commands::Command,decaf::util::concurrent::atomic::AtomicRefCounter> command={...})  Line 1077	C++
       	activemq-cpp.dll!activemq::transport::TransportFilter::onCommand(decaf::lang::Pointer<activemq::commands::Command,decaf::util::concurrent::atomic::AtomicRefCounter> command={...})  Line 91 + 0x27 bytes	C++
       	activemq-cpp.dll!activemq::transport::correlator::ResponseCorrelator::onCommand(decaf::lang::Pointer<activemq::commands::Command,decaf::util::concurrent::atomic::AtomicRefCounter> command={...})  Line 295	C++
       	activemq-cpp.dll!activemq::transport::TransportFilter::onCommand(decaf::lang::Pointer<activemq::commands::Command,decaf::util::concurrent::atomic::AtomicRefCounter> command={...})  Line 91 + 0x27 bytes	C++
       	activemq-cpp.dll!activemq::wireformat::openwire::OpenWireFormatNegotiator::onCommand(decaf::lang::Pointer<activemq::commands::Command,decaf::util::concurrent::atomic::AtomicRefCounter> command={...})  Line 145	C++
       	activemq-cpp.dll!activemq::transport::TransportFilter::onCommand(decaf::lang::Pointer<activemq::commands::Command,decaf::util::concurrent::atomic::AtomicRefCounter> command={...})  Line 91 + 0x27 bytes	C++
       	activemq-cpp.dll!activemq::transport::inactivity::InactivityMonitor::onCommand(decaf::lang::Pointer<activemq::commands::Command,decaf::util::concurrent::atomic::AtomicRefCounter> command={...})  Line 334	C++
       	activemq-cpp.dll!activemq::transport::TransportFilter::onCommand(decaf::lang::Pointer<activemq::commands::Command,decaf::util::concurrent::atomic::AtomicRefCounter> command={...})  Line 91 + 0x27 bytes	C++
       	activemq-cpp.dll!activemq::transport::IOTransport::fire(decaf::lang::Pointer<activemq::commands::Command,decaf::util::concurrent::atomic::AtomicRefCounter> command={...})  Line 116	C++
       	activemq-cpp.dll!activemq::transport::IOTransport::run()  Line 275	C++
       	activemq-cpp.dll!decaf::lang::Thread::run()  Line 143	C++
       	activemq-cpp.dll!`anonymous namespace'::runCallback(void * arg=0x02d9c3c8)  Line 266 + 0x11 bytes	C++
       	activemq-cpp.dll!`anonymous namespace'::threadEntryMethod(void * arg=0x02d9c3c8)  Line 254 + 0x15 bytes	C++
      

        Activity

        Hide
        Timothy Bish added a comment -

        Added some empty checks to try to workaround the problem but you'd be better off finding a fix for MS's bad STL in order to fully have confidence in the app.

        Show
        Timothy Bish added a comment - Added some empty checks to try to workaround the problem but you'd be better off finding a fix for MS's bad STL in order to fully have confidence in the app.
        Hide
        Christian Mamen added a comment - - edited

        Timothy Bish - Thanks, but i dont think ms stl is actually the root cause (sorry about opening the issue without better understanding whats going on).

        looking into the state i see that the connection have the following flags :

        +		started	{value=0x00000000 }	decaf::util::concurrent::atomic::AtomicBoolean
        +		closed	{value=0x00000000 }	decaf::util::concurrent::atomic::AtomicBoolean
        +		closing	{value=0x00000001 }	decaf::util::concurrent::atomic::AtomicBoolean
        

        another thread (an application thread) is currently destroying the connection, before attempting to reconnect

        pseudo code :

        [...]
            // delete resources (smart pointers)
            _consumer.reset();
            _producer.reset();
            
            _tempDestination->destroy(); // explicitly destroying the temporary destination
            _tempDestination.reset();
        
            _producer_session.reset()
            _consumer_session.reset();
           
            // delete and create a new connection
             scoped_ptr<cms::ConnectionFactory> connectionFactory(cms::ConnectionFactory::createCMSConnectionFactory( brokerUri ))
        
            _connection.reset( connectionFactory->createConnection() )
        [...]
        

        I suspect that the close method called by the destructor of ActiveMQConnection might have actually thrown an exception. the config might have been deleted at that point, whithout properly setting the flag, which should delete the head node of the map and set it to null (this is what i see with the debugger).

        ActiveMQConnection::~ActiveMQConnection() {
        
            try {
                this->close();
            }
            AMQ_CATCHALL_NOTHROW()
        
            try {
                // This must happen even if exceptions occur in the Close attempt.
                delete this->config;
            }
            AMQ_CATCHALL_NOTHROW()
        }
        

        (just a sample code from xtree, use by ms, _Myhead is actually the head node can only be null if the map is destroy)

        	void _Tidy()
        		{	// free all storage
        		erase(begin(), end());
        		this->_Alptr.destroy(&_Left(_Myhead));
        		this->_Alptr.destroy(&_Parent(_Myhead));
        		this->_Alptr.destroy(&_Right(_Myhead));
        		this->_Alnod.deallocate(_Myhead, 1);
        		_Myhead = 0, _Mysize = 0; 
        		}
        
        Show
        Christian Mamen added a comment - - edited Timothy Bish - Thanks, but i dont think ms stl is actually the root cause (sorry about opening the issue without better understanding whats going on). looking into the state i see that the connection have the following flags : + started {value=0x00000000 } decaf::util::concurrent::atomic::AtomicBoolean + closed {value=0x00000000 } decaf::util::concurrent::atomic::AtomicBoolean + closing {value=0x00000001 } decaf::util::concurrent::atomic::AtomicBoolean another thread (an application thread) is currently destroying the connection, before attempting to reconnect pseudo code : [...] // delete resources (smart pointers) _consumer.reset(); _producer.reset(); _tempDestination->destroy(); // explicitly destroying the temporary destination _tempDestination.reset(); _producer_session.reset() _consumer_session.reset(); // delete and create a new connection scoped_ptr<cms::ConnectionFactory> connectionFactory(cms::ConnectionFactory::createCMSConnectionFactory( brokerUri )) _connection.reset( connectionFactory->createConnection() ) [...] I suspect that the close method called by the destructor of ActiveMQConnection might have actually thrown an exception. the config might have been deleted at that point, whithout properly setting the flag, which should delete the head node of the map and set it to null (this is what i see with the debugger). ActiveMQConnection::~ActiveMQConnection() { try { this ->close(); } AMQ_CATCHALL_NOTHROW() try { // This must happen even if exceptions occur in the Close attempt. delete this ->config; } AMQ_CATCHALL_NOTHROW() } (just a sample code from xtree, use by ms, _Myhead is actually the head node can only be null if the map is destroy) void _Tidy() { // free all storage erase(begin(), end()); this ->_Alptr.destroy(&_Left(_Myhead)); this ->_Alptr.destroy(&_Parent(_Myhead)); this ->_Alptr.destroy(&_Right(_Myhead)); this ->_Alnod.deallocate(_Myhead, 1); _Myhead = 0, _Mysize = 0; }
        Hide
        Timothy Bish added a comment -

        Best to retest with v3.8.2 which was recently released. If you can reproduce then create a test case.

        Show
        Timothy Bish added a comment - Best to retest with v3.8.2 which was recently released. If you can reproduce then create a test case.
        Hide
        Christian Mamen added a comment - - edited

        Timothy Bish - Is it possible that the ActivemqConnection config->ActiveMQTempDestination needs synchronization during close()

        void ActiveMQConnection::close() {
           [...]
        
            // As TemporaryQueue and TemporaryTopic instances are bound to a connection
            // we should just delete them after the connection is closed to free up memory
            ArrayList<Pointer<ActiveMQTempDestination> > tempDests(this->config->activeTempDestinations.values());
            Pointer<Iterator<Pointer<ActiveMQTempDestination> > > iterator(tempDests.iterator());
        
            try {
                while (iterator->hasNext()) {
                    Pointer<ActiveMQTempDestination> dest = iterator->next();
                    dest->close();
                }
            } catch (cms::CMSException& error) {
                if (!hasException) {
                    ex = ActiveMQException(error.clone());
                    hasException = true;
                }
            }
        

        could become :

        void ActiveMQConnection::close() {
            [...]
                
            try {
                this->config->activeTempDestinations.lock();
                
                // As TemporaryQueue and TemporaryTopic instances are bound to a connection
                // we should just delete them after the connection is closed to free up memory
                ArrayList<Pointer<ActiveMQTempDestination> > tempDests(this->config->activeTempDestinations.values());
                Pointer<Iterator<Pointer<ActiveMQTempDestination> > > iterator(tempDests.iterator());
            
                while (iterator->hasNext()) {
                    Pointer<ActiveMQTempDestination> dest = iterator->next();
                    dest->close();
                }
                
                this->config->activeTempDestinations.unlock();
            } catch (Exception& error) {
                this->config->activeTempDestinations.unlock();
                if (!hasException) {
                    ex = ActiveMQException(error.clone());
                    hasException = true;
                }
            }
        

        One scenario that i have in mind is that Pointer<ActiveMQTempDestination> dest = iterator->next(); at some point (due to a race condition), returns null, and throws a decaf::lang::exceptions::NullPointerException

        since the try catch in the activemqconnection close() method only catch cms::exception, the exception is caugth lower

        Please Note :
        that i have not reproduce this error yet

        Show
        Christian Mamen added a comment - - edited Timothy Bish - Is it possible that the ActivemqConnection config->ActiveMQTempDestination needs synchronization during close() void ActiveMQConnection::close() { [...] // As TemporaryQueue and TemporaryTopic instances are bound to a connection // we should just delete them after the connection is closed to free up memory ArrayList<Pointer<ActiveMQTempDestination> > tempDests( this ->config->activeTempDestinations.values()); Pointer<Iterator<Pointer<ActiveMQTempDestination> > > iterator(tempDests.iterator()); try { while (iterator->hasNext()) { Pointer<ActiveMQTempDestination> dest = iterator->next(); dest->close(); } } catch (cms::CMSException& error) { if (!hasException) { ex = ActiveMQException(error.clone()); hasException = true ; } } could become : void ActiveMQConnection::close() { [...] try { this ->config->activeTempDestinations.lock(); // As TemporaryQueue and TemporaryTopic instances are bound to a connection // we should just delete them after the connection is closed to free up memory ArrayList<Pointer<ActiveMQTempDestination> > tempDests( this ->config->activeTempDestinations.values()); Pointer<Iterator<Pointer<ActiveMQTempDestination> > > iterator(tempDests.iterator()); while (iterator->hasNext()) { Pointer<ActiveMQTempDestination> dest = iterator->next(); dest->close(); } this ->config->activeTempDestinations.unlock(); } catch (Exception& error) { this ->config->activeTempDestinations.unlock(); if (!hasException) { ex = ActiveMQException(error.clone()); hasException = true ; } } One scenario that i have in mind is that Pointer<ActiveMQTempDestination> dest = iterator->next(); at some point (due to a race condition), returns null, and throws a decaf::lang::exceptions::NullPointerException since the try catch in the activemqconnection close() method only catch cms::exception, the exception is caugth lower Please Note : that i have not reproduce this error yet
        Hide
        Timothy Bish added a comment -

        Since the iterators are working with a snapshot of the original set of known temp destinations I don't think that is required here since the next() call should never return null.

        Show
        Timothy Bish added a comment - Since the iterators are working with a snapshot of the original set of known temp destinations I don't think that is required here since the next() call should never return null.
        Hide
        Christian Mamen added a comment - - edited

        I think your right - thanks for responding. and Pointer have a reference count, so the snapshot should be threadsafe

        My dmp files (when the crash occur) doesnt show anywhere that a thread was currently inside ActiveMQConnection::Close()

        Interestingly, i notice that the broker, at about the same time (~1msec) shows a bunch of java.net.SocketException: Connection reset

        2013-12-10 10:24:11,632 [ActiveMQ Transport: tcp:///127.0.0.1:55606@61616] WARN  org.apache.activemq.broker.TransportConnection.Transport - Transport Connection to: tcp://127.0.0.1:55606 failed: java.net.SocketException: Connection reset
        2013-12-10 10:24:29,459 [ActiveMQ Transport: tcp:///127.0.0.1:61992@61616] WARN  org.apache.activemq.broker.TransportConnection.Transport - Transport Connection to: tcp://127.0.0.1:61992 failed: java.net.SocketException: Connection reset
        2013-12-10 10:24:29,459 [ActiveMQ Transport: tcp:///127.0.0.1:61288@61616] WARN  org.apache.activemq.broker.TransportConnection.Transport - Transport Connection to: tcp://127.0.0.1:61288 failed: java.net.SocketException: Connection reset
        
        Show
        Christian Mamen added a comment - - edited I think your right - thanks for responding. and Pointer have a reference count, so the snapshot should be threadsafe My dmp files (when the crash occur) doesnt show anywhere that a thread was currently inside ActiveMQConnection::Close() Interestingly, i notice that the broker, at about the same time (~1msec) shows a bunch of java.net.SocketException: Connection reset 2013-12-10 10:24:11,632 [ActiveMQ Transport: tcp:///127.0.0.1:55606@61616] WARN org.apache.activemq.broker.TransportConnection.Transport - Transport Connection to: tcp://127.0.0.1:55606 failed: java.net.SocketException: Connection reset 2013-12-10 10:24:29,459 [ActiveMQ Transport: tcp:///127.0.0.1:61992@61616] WARN org.apache.activemq.broker.TransportConnection.Transport - Transport Connection to: tcp://127.0.0.1:61992 failed: java.net.SocketException: Connection reset 2013-12-10 10:24:29,459 [ActiveMQ Transport: tcp:///127.0.0.1:61288@61616] WARN org.apache.activemq.broker.TransportConnection.Transport - Transport Connection to: tcp://127.0.0.1:61288 failed: java.net.SocketException: Connection reset

          People

          • Assignee:
            Timothy Bish
            Reporter:
            Christian Mamen
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development