Made the SIP transactions list protected, added methods to add or remove transactions from list that lock the engine.

Remove transactions from engine's list on destroyed() as later access to their virtual methods is unsafe.


git-svn-id: http://voip.null.ro/svn/yate@2868 acf43c95-373e-0410-b603-e72c3f656dc1

libs/ysip/engine.cpp

@@ -142,10 +142,8 @@ SIPEvent::SIPEvent(SIPMessage* message, SIPTransaction* transaction)
 SIPEvent::~SIPEvent()
 {
     DDebug(DebugAll,"SIPEvent::~SIPEvent() [%p]",this);
-    if (m_transaction)
-	m_transaction->deref();
-    if (m_message)
-	m_message->deref();
+    TelEngine::destruct(m_transaction);
+    TelEngine::destruct(m_message);
 }
 
 
@@ -207,7 +205,7 @@ SIPTransaction* SIPEngine::addMessage(SIPMessage* message)
 	branch.clear();
     Lock lock(this);
     SIPTransaction* forked = 0;
-    ObjList* l = &TransList;
+    ObjList* l = &m_transList;
     for (; l; l = l->next()) {
 	SIPTransaction* t = static_cast<SIPTransaction*>(l->get());
 	if (!t)
@@ -258,7 +256,7 @@ bool SIPEngine::process()
 SIPEvent* SIPEngine::getEvent()
 {
     Lock lock(this);
-    ObjList* l = &TransList;
+    ObjList* l = &m_transList;
     for (; l; l = l->next()) {
 	SIPTransaction* t = static_cast<SIPTransaction*>(l->get());
 	if (t) {
@@ -270,7 +268,7 @@ SIPEvent* SIPEngine::getEvent()
 	    }
 	}
     }
-    for (l = &TransList; l; l = l->next()) {
+    for (l = &m_transList; l; l = l->next()) {
 	SIPTransaction* t = static_cast<SIPTransaction*>(l->get());
 	if (t) {
 	    SIPEvent* e = t->getEvent(false);

libs/ysip/transaction.cpp

@@ -67,7 +67,7 @@ SIPTransaction::SIPTransaction(SIPMessage* message, SIPEngine* engine, bool outg
     }
     m_invite = (getMethod() == "INVITE");
     m_state = Initial;
-    m_engine->TransList.append(this);
+    m_engine->append(this);
 }
 
 // Constructor from original and authentication requesting answer
@@ -104,13 +104,13 @@ SIPTransaction::SIPTransaction(SIPTransaction& original, SIPMessage* answer)
     // if this transaction is an INVITE and we append it to the list its
     //  ACK will be sent after the new INVITE which is legal but "unnatural"
     // some SIP endpoints seem to assume things about transactions
-    m_engine->TransList.append(this);
+    m_engine->append(this);
 #else
     // insert this transaction rather than appending it
     // this way we get a chance to send one ACK before a new INVITE
     // note that there is no guarantee because of the possibility of the
     //  packets getting lost and retransmitted or to use a different route
-    m_engine->TransList.insert(this);
+    m_engine->insert(this);
 #endif
 }
 
@@ -128,10 +128,10 @@ SIPTransaction::SIPTransaction(const SIPTransaction& original, const String& tag
 
 #ifdef SIP_PRESERVE_TRANSACTION_ORDER
     // new transactions at the end, preserve "natural" order
-    m_engine->TransList.append(this);
+    m_engine->append(this);
 #else
     // put new transactions first - faster to match new messages
-    m_engine->TransList.insert(this);
+    m_engine->insert(this);
 #endif
 }
 
@@ -140,15 +140,17 @@ SIPTransaction::~SIPTransaction()
 #ifdef DEBUG
     Debugger debug(DebugAll,"SIPTransaction::~SIPTransaction()"," [%p]",this);
 #endif
+    setPendingEvent(0,true);
+    TelEngine::destruct(m_lastMessage);
+    TelEngine::destruct(m_firstMessage);
+}
+
+void SIPTransaction::destroyed()
+{
+    DDebug(getEngine(),DebugAll,"SIPTransaction::destroyed() [%p]",this);
     m_state = Invalid;
-    m_engine->TransList.remove(this,false);
-    setPendingEvent();
-    if (m_lastMessage)
-	m_lastMessage->deref();
-    m_lastMessage = 0;
-    if (m_firstMessage)
-	m_firstMessage->deref();
-    m_firstMessage = 0;
+    m_engine->remove(this,false);
+    setPendingEvent(0,true);
 }
 
 const char* SIPTransaction::stateName(int state)
@@ -292,7 +294,7 @@ SIPEvent* SIPTransaction::getEvent(bool pendingOnly)
 	    // make sure we don't get trough this one again
 	    changeState(Invalid);
 	    // remove from list and dereference
-	    m_engine->TransList.remove(this);
+	    m_engine->remove(this);
 	    return e;
 	case Invalid:
 	    Debug(getEngine(),DebugFail,"SIPTransaction::getEvent in invalid state [%p]",this);
@@ -635,7 +637,7 @@ SIPEvent* SIPTransaction::getServerEvent(int state, int timeout)
 	    m_transmit = false;
 	    changeState(Invalid);
 	    // remove from list and dereference
-	    m_engine->TransList.remove(this);
+	    m_engine->remove(this);
 	    break;
 	case Trying:
 	    e = new SIPEvent(m_firstMessage,this);

libs/ysip/yatesip.h

@@ -814,6 +814,11 @@ protected:
      */
     SIPTransaction(SIPTransaction& original, SIPMessage* answer);
 
+    /**
+     * Pre-destruction notification used to clean up the transaction
+     */
+    virtual void destroyed();
+
     /**
      * Attempt to perform automatic client transaction authentication
      * @param answer SIP answer that creates the new transaction
@@ -1206,12 +1211,33 @@ public:
 	{ return m_allowed; }
 
     /**
-     * TransList is the key. 
-     * Is the list that holds all the transactions.
+     * Remove a transaction from the list
+     * @param transaction Pointer to transaction to remove
+     * @param deref Dereference the transaction if it was in list
      */
-    ObjList TransList;
+    inline void remove(SIPTransaction* transaction, bool deref = true)
+	{ lock(); m_transList.remove(transaction,deref); unlock(); }
+
+    /**
+     * Append a transaction to the end of the list
+     * @param transaction Pointer to transaction to append
+     */
+    inline void append(SIPTransaction* transaction)
+	{ lock(); m_transList.append(transaction); unlock(); }
+
+    /**
+     * Insert a transaction at the start of the list
+     * @param transaction Pointer to transaction to insert
+     */
+    inline void insert(SIPTransaction* transaction)
+	{ lock(); m_transList.insert(transaction); unlock(); }
 
 protected:
+    /**
+     * The list that holds all the SIP transactions.
+     */
+    ObjList m_transList;
+
     u_int64_t m_t1;
     u_int64_t m_t4;
     unsigned int m_maxForwards;