On 04/29/2011 05:18 PM, Domenico Chierico wrote:
> On Fri, 2011-04-29 at 11:48 +0800, Joegen Baclor wrote:
>> Domenico,
>>
>> Let me see if I got this right. Reading the implementation of
>> SipProtocolServerBase::handleMessage(OsMsg& eventMessage) the only
>> event it is concerned with is of subtype
>> SipProtocolServerBase::SIP_SERVER_GC it eventually calls
>> removeOldClients(0);
>>
>> Grepping for removeOldClients in the entire sipXtackLib, it is already
>> called within SipUserAgent::garbageCollection() method which is called
>> from SipUserAgent::handleMessage(). I believe that this should be the
>> only place where garbage collection must be performed to avoid mutex
>> contentions or worse, deadlocks.
>>
>> My recommendation is to still override the run method of the
>> SipTcpServer but to simply consume the event message without calling
>> handleMessage. This will make it evident to delevelopers looking at
>> the code later on that the run loop of the TCP server is meant to not
>> handle anything (as it is now). The second recommendation is (I know
>> you already suggested this to George) that the SipClient should never
>> post a SipProtocolServerBase::SIP_SERVER_GC and instead rely on the
>> centralized garbage collection performed by the
>> SipUserAgent::garbageCollect() method.
>>
>> Infact, the comment in SipClient post event is quite scary.
>>
>> if (mpSipServer)
>> {
>> OsMsg message(OsMsg::OS_EVENT,
>> SipProtocolServerBase::SIP_SERVER_GC);
>> // If the SipServer's queue is full, don't wait, since
>> // that can cause deadlocks. The SipServer will eventually
>> // garbage-collect terminated clients spontaneously.
>> mpSipServer->postMessage(message,
>> OsTime::NO_WAIT);
>> }
>>
>> Comments?
>>
> I've made the patch but now I have just a little doubt about the impact that
> this will have on performance.
>
> Now we are calling GC for collecting only on time base (this for both Tcp and
> Udp Servers). Can this have an impact on overall performance of the proxy.
>
> Is not more sure make SipTcpServer behave as SipUdpServer that is proved to
> work?
>
> Maybe you have a deeper knowledge on this.
>
> thanks
> Domenico Chierico
It's the way it works right now and I don't think its causing any memory
issues. So I think the safest move (might not be the most elegant) is
to preserve the status quo and just get rid of the nasty queue is full
error message.
_______________________________________________
sipx-dev mailing list
[email protected]
List Archive: http://list.sipfoundry.org/archive/sipx-dev/
