[asterisk-scf-commits] asterisk-scf/release/sip.git branch "master" updated.

[Commits to the Asterisk SCF project code repositories]

branch "master" has been updated
       via  447c46a76ebf4f537f302213e35195b3f04aef48 (commit)
      from  58cc573539da83eb5bf2fc45c404fdfbf88560ec (commit)

Summary of changes:
 src/SIPSession.cpp |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 85 insertions(+), 2 deletions(-)


- Log -----------------------------------------------------------------
commit 447c46a76ebf4f537f302213e35195b3f04aef48
Author: Brent Eagles <beagles at digium.com>
Date:   Fri Jun 15 14:29:42 2012 -0230

    Add a fix to get around a race condition resulting in pjsip flip/flopping
    SDP session related pools while we are using them in another thread. The
    fix involves creating a separate SDP pool in SIPSession for its own use and
    managing it in sync with its own requirements.

diff --git a/src/SIPSession.cpp b/src/SIPSession.cpp
index aabfde6..a963e9a 100755
--- a/src/SIPSession.cpp
+++ b/src/SIPSession.cpp
@@ -145,11 +145,20 @@ public:
           mAdapter(adapter), mDialog(0), mInviteSession(0), mEndpoint(endpoint), mDestination(destination),
           mManager(manager), mServiceLocator(serviceLocator), mReplicationContext(replicationContext), 
           mNatOptions(natOptions), mSDP(0), 
-          mOperationContextCache(OperationContextCache::create(180))
+          mOperationContextCache(OperationContextCache::create(180)),
+          mSDPPool(0)
     { 
 
     }
 
+    ~SIPSessionPriv()
+    {
+        if (mSDPPool)
+        {
+            pj_pool_release(mSDPPool);
+        }
+    }
+
 
     void removeListener(const AsteriskSCF::SessionCommunications::V1::SessionListenerPrx& listener)
     {
@@ -353,9 +362,78 @@ public:
 
     AsteriskSCF::Operations::OperationContextCachePtr mOperationContextCache;
 
+    /**
+     * Get our pjsip memory pool for the current SDP negotiation session. Using an
+     * independent pool prevents memory growth for long running sessions as the pool
+     * is reset whenever starting a new SDP session. Of course, this means you should
+     * not cache pointers to memory in the pool outside the context of an SDP 
+     * negotiation session.
+     *
+     * WARNING: All of SIPSession's operations that appear to be related to SDP
+     * negotiation are not necessarily called during an SDP negotiation session
+     * (they are called before). This pool is not (and probably should not) be
+     * used in these places at the moment. If the code changes in the future to
+     * break the "in process" SDP negotation related code out of the standard
+     * initialization code, then this could be used there as well if necessary.
+     */
+    pj_pool_t* getSDPPool()
+    {
+        //
+        // We should not be looking at SDP pool if we are not within and invite.
+        //
+        if (!mInviteSession || !mInviteSession->dlg)
+        {
+            return 0;
+        }
+
+        if (!mSDPPool)
+        {
+            string sdpPoolName = mEndpoint->getName() + ".sdp.pool";
+            mSDPPool = pjsip_endpt_create_pool(
+                mInviteSession->dlg->endpt, 
+                sdpPoolName.c_str(),
+                256, 
+                256);
+        }
+        return mSDPPool;
+    }
+
+    /**
+     * Reset our internal SDP pool. Should be called when starting SDP
+     * negotiation.  So far, the only place where we use it is in responding to
+     * an offer. 
+     *
+     * WARNING: All of SIPSession's operations that appear to be related to SDP
+     * negotiation are not necessarily called during an SDP negotiation session
+     * (they are called before). This pool is not (and probably should not) be
+     * used in these places at the moment. If the code changes in the future to
+     * break the "in process" SDP negotation related code out of the standard
+     * initialization code, then this could be used there as well if necessary.
+     */
+    void resetSDPPool()
+    {
+        pj_pool_t* t = getSDPPool();
+        if (t)
+        {
+            pj_pool_reset(t);
+        }
+    }
+
 private:
     static ReadOnlyCookieTypes mReadOnlyCookieTypes;
 
+    //
+    // There is race condition in pjsip with the memory pool flip/flopping
+    // before and after negotiation. It is not safe when a separate worker thread
+    // handles the SDP negotiation operations (as it does here).  The really
+    // safe thing to do is to have our own memory pool for SDP related stuff
+    // and reset it whenever we enter and SDP call flow. These calls cannot (or
+    // should not) be made recursively for a given session, so contention and
+    // races should not be an issue. The data member is accessed through
+    // accessors to prevent repetition of boilerplate.
+    //
+    pj_pool_t* mSDPPool;
+
 }; // class SIPSessionPriv
 typedef boost::shared_ptr<SIPSessionPriv> SIPSessionPrivPtr;
 
@@ -3789,6 +3867,11 @@ pjmedia_sdp_session *SIPSession::createSDPAnswer(
     const pjmedia_sdp_session* offer,
     AsteriskSCF::Media::V1::StreamInformationDict& newStreams)
 {
+    //
+    // Entering a new SDP negotiation, restart our pool.
+    //
+    mImplPriv->resetSDPPool();
+
     // Create the most common part of the SDP if not already done
     if (!mImplPriv->mSDP)
     {
@@ -3878,7 +3961,7 @@ pjmedia_sdp_session *SIPSession::createSDPAnswer(
                                                      &offer->media[stream]->desc.fmt[format])))
             {
                 pjmedia_sdp_rtpmap *rtpmap;
-                if ((pjmedia_sdp_attr_to_rtpmap(mImplPriv->mInviteSession->pool_active, attr, &rtpmap)) == PJ_SUCCESS)
+                if ((pjmedia_sdp_attr_to_rtpmap(mImplPriv->getSDPPool(), attr, &rtpmap)) == PJ_SUCCESS)
                 {
                     descriptor->subtype = std::string(pj_strbuf(&rtpmap->enc_name), pj_strlen(&rtpmap->enc_name));
                     descriptor->samplerate = rtpmap->clock_rate;

-----------------------------------------------------------------------


-- 
asterisk-scf/release/sip.git