RestComm / jain-sip.ha

JAIN-SIP-HA : Provides extensions done by TeleStax for high availability and fault tolerance through replication of various states of the stack. It supports Call Established Failover or Early Dialog Failover.
http://www.restcomm.com/
GNU Affero General Public License v3.0
8 stars 21 forks source link

[MemoryLeaks] Transactions didn't remove from Cache if it is in LOCAL mode #12

Open SergeyLee opened 8 years ago

SergeyLee commented 8 years ago

These Memory Leaks appeared after Performance Tests with SIP UAS Example with JBoss DEFAULT profile.

Here ClusteredSipStackImpl created Transactions: https://github.com/RestComm/jain-sip.ha/blob/master/core/src/main/java/org/mobicents/ha/javax/sip/ClusteredSipStackImpl.java#L561-L576

If sipCache.inLocalMode() or transactionFactory == null we got jSIP-Ext SipStackImpl https://github.com/RestComm/jain-sip.ext/blob/master/src/main/java/org/mobicents/ext/javax/sip/SipStackImpl.java#L156-L171

In our case we have created transactionFactory as MobicentsHATransactionFactory, because replicationStrategy == ReplicationStrategy.EarlyDialog https://github.com/RestComm/jain-sip.ha/blob/master/core/src/main/java/org/mobicents/ha/javax/sip/ClusteredSipStackImpl.java#L110-L118

So transactionFactory != null and we created Transactions with MobicentsHATransactionFactory and they added to Cache: https://github.com/RestComm/jain-sip.ha/blob/master/core/src/main/java/gov/nist/javax/sip/stack/MobicentsHASIPClientTransaction.java#L125 https://github.com/RestComm/jain-sip.ha/blob/master/core/src/main/java/gov/nist/javax/sip/stack/MobicentsHASIPServerTransaction.java#L130

When ClusteredSipStackImpl removed Transaction in LOCAL mode it didn't remove Transaction from Cache: https://github.com/RestComm/jain-sip.ha/blob/master/core/src/main/java/org/mobicents/ha/javax/sip/ClusteredSipStackImpl.java#L474-L476 and here: https://github.com/RestComm/jain-sip.ha/blob/master/core/src/main/java/org/mobicents/ha/javax/sip/ClusteredSipStackImpl.java#L499-L501

SergeyLee commented 8 years ago

I suggest to add new condition:

if(sipCache.inLocalMode() && transactionFactory == null) {
    return;
}

Then Transaction removed from Cache.

SergeyLee commented 8 years ago

Perhaps we don't need Cache in LOCAL mode.

jaimecasero commented 8 years ago

I agree add/remove conditions needs to be consistent, so the "same" condition must be used for add and remove, regardless any environment configuration. We cant allow those leaks to happen in any situation...

SergeyLee commented 8 years ago

@jaimecasero is it necessary to set wrappedTransaction to null, and eventFiringAddress to null for clearing? wdyt?

https://github.com/RestComm/jain-slee.sip/blob/master/resources/sip11/ra/src/main/java/org/mobicents/slee/resource/sip11/wrappers/ClientTransactionWrapper.java#L215 https://github.com/RestComm/jain-slee.sip/blob/master/resources/sip11/ra/src/main/java/org/mobicents/slee/resource/sip11/wrappers/ServerTransactionWrapper.java#L217

jaimecasero commented 8 years ago

Hi, If we manage to free the reference to the parent object (by removing the wrapper from the cache), there is no need to go on freeing internal object references, they will be reclaimed naturally by GC when root object is freed.

Regards

On Fri, Jun 24, 2016 at 10:43 AM, SergeyLee notifications@github.com wrote:

@jaimecasero https://github.com/jaimecasero is it necessary to set wrappedTransaction to null, and eventFiringAddress to null for clearing? wdyt?

https://github.com/RestComm/jain-slee.sip/blob/master/resources/sip11/ra/src/main/java/org/mobicents/slee/resource/sip11/wrappers/ClientTransactionWrapper.java#L215

https://github.com/RestComm/jain-slee.sip/blob/master/resources/sip11/ra/src/main/java/org/mobicents/slee/resource/sip11/wrappers/ServerTransactionWrapper.java#L217

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RestComm/jain-sip.ha/issues/12#issuecomment-228290682, or mute the thread https://github.com/notifications/unsubscribe/AHe9SfX48YRtJo1J_Qvwb-9dvD7DfM4pks5qO5i_gaJpZM4I8013 .