RestComm / Restcomm-Connect

The Open Source Cloud Communications Platform
http://www.restcomm.com/
GNU Affero General Public License v3.0
241 stars 215 forks source link

Restcomm conference with music on hold leave RMS resources open #1612

Closed gvagenas closed 7 years ago

gvagenas commented 7 years ago

To reproduce dial 1310 RC is not deleting the bridge/1 nor the ivr/1 (and it’s connections) only see 4 DLCX commands

  1. delete connection between bridge/1 and cnf/1
  2. delete connection between cnf/1 and bridge/1
  3. delete cnf/1 endpoint (and it’s connections)
  4. delete ivr/2 endpoint (and it’s connections)

it missing two DLCX, one for ivr/1 and other for bridge/1

Released local connection,pool size:100,free:98 the 2 missing local connections are the ones between bridge/1 and ivr/1 Released player,pool size:50,free:49 it’s the player from ivr/1

maria-farooq commented 7 years ago

@gvagenas to reproduce do we need to hangup while on music on hold right? does it happen when we are not on MOH, for example when we have a moderator present and client hangsup normally?

gvagenas commented 7 years ago

@maria-farooq right, you hangup when listening MOH.

Didn't checked for when the moderator is present

maria-farooq commented 7 years ago

@gvagenas i reproduced with and without moderator present: in both cased initial bridge and corresponding ivr endpoints are not deleted. i am working on debugging it more and on solution

maria-farooq commented 7 years ago

@gvagenas @deruelle @hrosa After another level of investigation i found out that problem is due to Faulty FSM of Media-Server Controller for a Call: [MMSCallController](https://github.com/RestComm/Restcomm-Connect/blob/master/restcomm/restcomm.mscontrol.mms/src/main/java/org/restcomm/connect/mscontrol/mms/MmsCallController.java)

OnLeave -transition-> closingInternalLink and thats it in short Call Controller assumes that on an event of Leaving a Conference, it should only remove the link between bridge-ep and conference-ep and nothing else.

@gvagenas @hrosa can you please help me understand why is this like it?

maria-farooq commented 7 years ago

Here is discussion and me and @hrosa had on Slack:

Quoting my self

please check this https://github.com/RestComm/Restcomm-Connect/blob/master/restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/Call.java#L2028 Left is an event sent by MMSCallController when it has removed the link b/w bridge—and— conference EP here https://github.com/RestComm/Restcomm-Connect/blob/master/restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/Call.java#L2039 we see call is moved to completed (edited) as i suggested earlier it should not go to completed just like that :smile: bcz there are resources (bridge and ivr) which need cleanup (edited) it should send CloseMediaSession to MMSCallController so it would clean bridge and ivr EPs would you agree?

quoting @hrosa :

Should move to stopping. If call is not LCM move to stopping after leaving conf, Then MS controller close media session And call moves to completed. Sounds good

gvagenas commented 7 years ago

@maria.farooq The Call must not move to stopping, its the VoiceInterpreter that should decide if the Call should stop or not. Moving the Call to Completed after the Left message arrives is because we need to notify the VoiceInterpreter that the Call left the conference and wait for the next step from VI.

To me bug is on the VoiceInterprepter or Conference that doesn't act on the Left and Completed states. You should check what is the state of VI here https://github.com/RestComm/Restcomm-Connect/blob/ea84703af01b780ad8fb986ff4fe4c1f5f8ecd00/restcomm/restcomm.interpreter/src/main/java/org/restcomm/connect/interpreter/VoiceInterpreter.java#L1164-L1164

maria-farooq commented 7 years ago

i am not sure why this else is missing parenthesis

gvagenas commented 7 years ago

@maria-farooq if you ignore the comment at line 1136 (https://github.com/RestComm/Restcomm-Connect/blob/master/restcomm/restcomm.interpreter/src/main/java/org/restcomm/connect/interpreter/VoiceInterpreter.java#L1136) the statement is:

else if (is(forking) && ((dialBranches != null && dialBranches.contains(sender)) || outboundCall == null)) {

which is legit and ok.

George