Skip to content

UserAgentManager actor restarts on NPE after ApplicationSession invalidate() #1764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gvagenas opened this issue Feb 3, 2017 · 11 comments
Closed

Comments

@gvagenas
Copy link
Contributor

gvagenas commented Feb 3, 2017

  • change pong() method to setInvalidateWhenReady(true)
  • override postStop() method and print useful information there such as the last state, number of registrations or anything else you think it will help to debug that
@gvagenas gvagenas added the 1. Bug label Feb 3, 2017
@gvagenas gvagenas added this to the 8.1.0 milestone Feb 3, 2017
maria-farooq pushed a commit that referenced this issue Feb 8, 2017
@deruelle
Copy link
Member

@gvagenas @maria-farooq Should this be closed ?

@gvagenas
Copy link
Contributor Author

gvagenas commented Feb 20, 2017

@deruelle can you shed some light for SipApplicationSession invalidation?
Here

we try to set InvalidateWhenReady with all the precautions but still I can see we get exceptions:

09:29:36,639 INFO  [org.restcomm.connect.telephony.ua.UserAgentManager] (RestComm-akka.actor.default-dispatcher-1724) UserAgentManager Processing Message: "org.mobicents.servlet.sip.message.Servlet3SipServletRequestImpl sender : class akka.actor.DeadLetterActorRef self is terminated: false
09:29:36,659 INFO  [org.restcomm.connect.telephony.ua.UserAgentManager] (RestComm-akka.actor.default-dispatcher-1724) Client in front of LB. Patching URI: 
09:29:36,660 INFO  [org.restcomm.connect.monitoringservice.MonitoringService] (RestComm-akka.actor.default-dispatcher-1724) MonitoringService Processing Message: "org.restcomm.connect.telephony.api.UserRegistration sender : class akka.actor.RepointableActorRef self is terminated: false
09:29:36,683 INFO  [org.restcomm.connect.telephony.ua.UserAgentManager] (RestComm-akka.actor.default-dispatcher-1724) The user agent manager updated  at address 
09:29:36,684 INFO  [org.mobicents.servlet.sip.core.session.SipSessionImpl] (MSS-Executor-Thread-4) Invalidating the sip session (817079472;1540352584@192_168_0_17;980ae4db;RestComm)
09:29:36,684 INFO  [gov.nist.javax.sip.stack.SIPTransactionStack] (RestComm-akka.actor.default-dispatcher-1735) <message

09:29:36,684 INFO  [org.mobicents.servlet.sip.core.session.SipApplicationSessionImpl] (MSS-Executor-Thread-4) Invalidating the following sip application session 980ae4db;RestComm
09:29:36,684 INFO  [org.mobicents.servlet.sip.core.session.SipApplicationSessionImpl] (MSS-Executor-Thread-4) The following sip application session 980ae4db;RestComm has been invalidated
09:29:36,684 ERROR [org.restcomm.connect.telephony.ua.UserAgentManager] (RestComm-akka.actor.default-dispatcher-1724) Exception while trying to setInvalidateWhenReady(true) for application session, exception: java.lang.IllegalStateException: SipApplicationSession already invalidated !

@deruelle
Copy link
Member

@jaimecasero is responsible for anything related to SIP Servlets. My guess is that akka threading makes it so that the SipApplicationSession concurrency is not respected anymore as business logic runs in an akka thread and not in a container thread...

@jaimecasero
Copy link
Contributor

in this case the appsession seems to be invalidated already (java.lang.IllegalStateException: SipApplicationSession already invalidated !). whether the app invalidated the session previously, or the container invalidated the session, I cant check on this short log...

@gvagenas
Copy link
Contributor Author

@jaimecasero if you check the link I provided in the comment earlier (

) you will see that before trying to setInvalidateWhenReady(true) we check:

if (request.getApplicationSession() != null) {
                    if (request.getApplicationSession().isValid()) {

So looks like a race condition.

@jaimecasero
Copy link
Contributor

as Jean mentioned, container Threads maybe racing with Akka Threads.

We need to elaborate on how to sync Akka Threads and SipServlets threads. For me there are two possible options:

  1. Let SipServlets and Akka use the same threadPool and dispatching policy. I guess Akka allows to set the executorcontext, and then through some SipServlet extension we could potentially tell Akka to use SipServlets threadpool. This solution is dangerous for both sides...
  2. Let Restcomm Akka actors use the SipServlets Async mode for everything related to SIPServlets API usage (https://mobicents.ci.cloudbees.com/job/RestcommSipServlets-Release/lastSuccessfulBuild/artifact/documentation/jsr289-extensions-apidocs/index.html). This may require more backAndForth between the actor logic, and the AsyncWork task logic, since you can run all at once (seems proper eventDriven programmaing anyway). Note the Aync logic is executed in the context of a particular appSession, so all Restcomm actors should store the appSession id, in their context (probably already there). When SIPServlets API needs to be invoked(from restcomm akka actor), you need to use the SipSessionsUtilExt to look up the sessions, and proceed to the Async mode invocation. Notice this is what you do now on the other way around, from SIPServlets to Akka, probably you have some Akka facility that enables you to inject akka events from sipservlets code, so the Akka MessageDiapatcher may apply its policy...

Again,first approach will require less app changes, but it may raise its own issues. Second approach may invovle lots of changes tin Restcomm actors, but it will definately work, since AsyncMode will invoke the SIPServlets concurrency mode...

thoughts?

@gvagenas
Copy link
Contributor Author

@jaimecasero we can have actors to hold the app session id, this is not a problem.
For the AsyncWork you mention on solution (2) we will need to change the FSM of actors but sounds more safe approach as you mention.
The AsyncWork will be used even to ask invalidation of a session?
Can you point me to any examples for SipApplicationSessionAsynchronousWork ?

@jaimecasero
Copy link
Contributor

jaimecasero commented Feb 20, 2017

I know it sounds tedious to go through this 2 approch just to invalidate a session. We need to notice Akka and SipServlets are compeltely different eventDriven frameworks, and this requires careful cooperation.

Can you imagine a SipServlets method invoking AkkaActor logic without going through the proper Akka MessageDispather, it would be a mess,right?. Even something as simple as closing an Akka Actor,requires the Akka framework to be managing that activity,right? same with SIPServlets. The fact the SIPServlects API is available anywhere in your application through the classloaders, doesnt mean it will work without proper management. Notice in JEE world the application is not meant to create its own ThreadPool, becuase those are not managed by container, so behavior will always be unpredictable under this nonStandard deployment...

This is the class you need to use https://mobicents.ci.cloudbees.com/job/RestcommSipServlets-Release/lastSuccessfulBuild/artifact/documentation/jsr289-extensions-apidocs/org/mobicents/javax/servlet/sip/SipSessionAsynchronousWork.html to create an Async task. And this the one to execute it https://mobicents.ci.cloudbees.com/job/RestcommSipServlets-Release/lastSuccessfulBuild/artifact/documentation/jsr289-extensions-apidocs/org/mobicents/javax/servlet/sip/SipSessionsUtilExt.html#scheduleAsynchronousWork(java.lang.String,%20org.mobicents.javax.servlet.sip.SipApplicationSessionAsynchronousWork)

Example of full usage at https://github.com/RestComm/sip-servlets/blob/master/sip-servlets-test-suite/applications/click-to-call-servlet/src/main/java/org/mobicents/servlet/sip/testsuite/SimpleWebServlet.java#L182

@gvagenas
Copy link
Contributor Author

Thanks @jaimecasero , I think I will create a utility class or actor that will take care all the SipServlets operations. Other actors will dispatch SipServlet tasks to this new actor so the changes in FSM will be less.

@jaimecasero
Copy link
Contributor

sure, if you manage to create a simple wrapper to inject that logic it will be easier.

Not sure how the SipServletsAPI invocation is merged in the regular RestcommActor logic. So, results from current invocations to SipServlets API may require additional work to be sent again to Akka,through proper Event passing...

If you decide to go this way, and since you will need to review all SIPServlets invocation stuff, it might be good chance to take into consideration the new feature we are about to introduce in SIPServlets. The nonBlocking connect. This will be a major enhancement for Restcomm in terms of performance, and specially interesting in a Cluod environment to reduce blockign threads on network connect attempts. The basic change for you is that anytime you call message.send, instead of receiving a sync IOException in the same thread, you will receive a 503 response (in case of requests) later. It is up to you if you like to consider this now,or later, but probably @deruelle will require this as part of Restcomm sooner or later. Maybe you can just consider and make assumptions on what will be required, so when sipservlets is ready, you can perform the actual changes...

Regards

@gvagenas
Copy link
Contributor Author

Closed by #2023

@gvagenas gvagenas closed this as completed Jun 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants