[MgcpDriver] transactionHandlerTimer not cancelled on Driver shutdown

[skywalker788]

Hello everyone,
I'm using the MGCP Driver in my SipServlet project.
Symilar to what happens with the issue #627 we recently close, I found another Thread that does not terminate on Driver shutdown.
It is the transactionHandlerTimer that is allocated as static member by the abstract class TransactionHandler.

** Expiration timer */
protected static Timer transactionHandlerTimer = new Timer("TransactionHandlerTimer");

As you can verify this Timer is shared between all the TransactionHandler implementations and it is not cancelled on Driver shutdown so it never ends. This is very likely to create a memory leak if the operation is repeated several times (e.g. creating and destroying driver for some reason. In my scenario it happens when I perform an hot deploy of my SipServlet Application in the SipServlet container).

I could propose two solutions:

• The naive solution is to create a method in the transaction Handler to cancel the timer and invoke it on the Driver shutdown from JainMgcpStackProviderImpl class. I think that it is not an elegant solution because it is not a provider responsability to cancel a timer allocated somewhere else in the code.

• Probably it would be better that is the provider to allocate in a static way this Timer and cancel it on shutdown.

Tell me what you think. I can collaborate on this with a PR.

GS

[hrosa]

Hi @skywalker788 , agree with your comments. It's not the TransactionHandler responsibility to stop a shared Timer. I think it would also be better to replace that Timer with a ScheduledExecutorService, which is more versatile and can help us reuse a thread pool.

Probably it would be better that is the provider to allocate in a static way this Timer and cancel it on shutdown.

Agree, but not sure if it is necessary for the Timer to be exposed statically... it's ugly.
Probably the JainMgcpStackImpl own the Timer (and be responsible for its cancellation) and pass it down to the MessageHandler, which will provide the Timer to every TransationHandler. Wdyt?

Thanks for taking ownership of this one as well :)

Cheers,

• H

[skywalker788]

Hi @hrosa,
Agree with you, probably it's the better solution.

I am just investigating on the "symptoms" because unluckly I've not the time to study the entire project design. The solution I propose was in my opinion the one with less impact on the project.

If all the transaction handlers are managed by JainMgcpStackImpl your suggestion is on the right way.

I will work on this asap.

Thanks for the fast fb

GS

[gsaslis]

Hey @skywalker788,

Wondering if you found some time to work on this one in the end?
Thanks in advance

[skywalker788]

@gsalis Too busy in the last months... I hope in the next days

[gsaslis]

@skywalker788 i know exactly what you mean... :/

Plz feel free to ping me or @hrosa if you need any help on this. ; )