Browse code

b/f: several minor bugs in session creation.

- the session pointer should not be touched after the session has been started (could be immediately destroyed).
- return 481 on CANCEL if the transaction cannot be found.
- reply properly with 482 if the UAS dialog already exists.

Raphael Coeffic authored on 24/03/2011 19:11:24
Showing 11 changed files
... ...
@@ -282,13 +282,13 @@ EXEC_ACTION_START(DLGDialoutAction) {
282 282
  
283 283
   DBG("sess_params: '%s'\n", AmArg::print(*sess_params).c_str());
284 284
 
285
-  AmSession* new_sess = AmUAC::dialout(user, app_name, r_uri, from, from_uri, to, ltag, hdrs, sess_params);
286
-
287
- if (NULL != new_sess) {
288
-   sc_sess->var[arrayname + "_ltag"] = new_sess->getLocalTag();
289
- } else {
290
-   sc_sess->var[arrayname + "_ltag"] = "";
291
-   sc_sess->SET_ERRNO(DSM_ERRNO_GENERAL);
292
- }
285
+  string new_sess_tag = AmUAC::dialout(user, app_name, r_uri, from, from_uri, to, ltag, hdrs, sess_params);
286
+
287
+  if (!new_sess_tag.empty()) {
288
+    sc_sess->var[arrayname + "_ltag"] = new_sess_tag;
289
+  } else {
290
+    sc_sess->var[arrayname + "_ltag"] = "";
291
+    sc_sess->SET_ERRNO(DSM_ERRNO_GENERAL);
292
+  }
293 293
 
294 294
 } EXEC_ACTION_END;
... ...
@@ -255,7 +255,7 @@ void CallGenFactory::createCall(const AmArg& args) {
255 255
   AmArg* c_args = new AmArg(args);
256 256
   
257 257
   DBG("placing new call to %s\n", call_ruri.c_str());
258
-  /* AmSession* s = */ AmUAC::dialout("callgen", // user
258
+  /* string tag = */ AmUAC::dialout("callgen", // user
259 259
 				APP_NAME,  
260 260
 				call_ruri,
261 261
 				"<" + from +  ">", from, 
... ...
@@ -132,11 +132,11 @@ string DIDial::dialout(const string& application,
132 132
   DBG("dialout application '%s', user '%s', from '%s', to '%s'\n", 
133 133
       application.c_str(), user.c_str(), from.c_str(), to.c_str());
134 134
 
135
-  AmSession* s = AmUAC::dialout(user.c_str(), application,  to,  
135
+  string tag = AmUAC::dialout(user.c_str(), application,  to,  
136 136
 				"<" + from +  ">", from, "<" + to + ">", 
137 137
 				string(""), string(""), extra_params);
138
-  if (s)
139
-    return s->getLocalTag();
138
+  if (!tag.empty())
139
+    return tag;
140 140
   else 
141 141
     return "<failed>";  
142 142
 }
... ...
@@ -162,15 +162,15 @@ string DIDial::dialout_auth(const string& application,
162 162
   } else {
163 163
     a->setBorrowedPointer(new UACAuthCred(a_realm, a_user, a_pwd));
164 164
   }
165
-  AmSession* s = AmUAC::dialout(user.c_str(), application,  to,  
165
+  string tag = AmUAC::dialout(user.c_str(), application,  to,  
166 166
 				"<" + from +  ">", from, "<" + to + ">", 
167 167
 				string(""), // callid
168 168
 				string(""), // xtra hdrs
169 169
 				a);
170 170
   delete a;
171 171
 
172
-  if (s)
173
-    return s->getLocalTag();
172
+  if (!tag.empty())
173
+    return tag;
174 174
   else 
175 175
     return "<failed>\n";
176 176
 }
... ...
@@ -195,14 +195,14 @@ string DIDial::dialout_auth_b2b(const string& application,
195 195
   a.push(a_pwd.c_str());
196 196
   a.push(callee_ruri.c_str());
197 197
 
198
-  AmSession* s = AmUAC::dialout(
198
+  string tag = AmUAC::dialout(
199 199
       announcement.c_str(), application,  caller_ruri,
200 200
       "<" + from +  ">", from, "<" + to + ">", 
201 201
       string(""), // callid
202 202
       string(""), //xtra hdrs
203 203
       &a);
204
-  if (s)
205
-    return s->getLocalTag();
204
+  if (!tag.empty())
205
+    return tag;
206 206
   else 
207 207
     return "<failed>\n";
208 208
 }
... ...
@@ -223,7 +223,7 @@ string DIDial::dialout_pin(const string& application,
223 223
 					  it->second.user, 
224 224
 					  it->second.pwd));
225 225
     
226
-    AmSession* s = AmUAC::dialout(user.c_str(), application,  
226
+    string tag = AmUAC::dialout(user.c_str(), application,  
227 227
 				  "sip:"+to_user+"@"+it->second.realm,  
228 228
 				  "<sip:" + it->second.user+"@"+it->second.realm + ">", 
229 229
 				  "sip:"+it->second.user+"@"+it->second.realm, 
... ...
@@ -231,8 +231,8 @@ string DIDial::dialout_pin(const string& application,
231 231
 				  string(""), // callid
232 232
 				  string(""), // xtra hdrs
233 233
 				  a);
234
-    if (s)
235
-      return s->getLocalTag();
234
+    if (!tag.empty())
235
+      return tag;
236 236
     else 
237 237
       return "<failed>\n";
238 238
   } else 
... ...
@@ -49,7 +49,6 @@ static PyObject* IvrUAC_dialout(IvrUAC* self, PyObject* args)
49 49
 		       &from, &from_uri, &to))
50 50
     return NULL;
51 51
 
52
-  //AmSession* newSession = 
53 52
   AmUAC::dialout(user, app_name, r_uri, 
54 53
 		 from, from_uri, to);
55 54
 
... ...
@@ -731,13 +731,12 @@ void WebConferenceFactory::dialout(const AmArg& args, AmArg& ret) {
731 731
   AmArg* a = new AmArg();
732 732
   a->setBorrowedPointer(new UACAuthCred("", auth_user, auth_pwd));
733 733
 
734
-  AmSession* s = AmUAC::dialout(room.c_str(), APP_NAME,  to,  
735
-				"<" + from +  ">", from, "<" + to + ">", 
736
-				string(""), // callid
737
-				headers,    // headers
738
-				a);
739
-  if (s) {
740
-    string localtag = s->getLocalTag();
734
+  string localtag = AmUAC::dialout(room.c_str(), APP_NAME,  to,  
735
+				   "<" + from +  ">", from, "<" + to + ">", 
736
+				   string(""), // callid
737
+				   headers,    // headers
738
+				   a);
739
+  if (!localtag.empty()) {
741 740
     ret.push(0);
742 741
     ret.push("OK");
743 742
     ret.push(localtag.c_str());
... ...
@@ -205,23 +205,38 @@ void AmSessionContainer::destroySession(AmSession* s)
205 205
     }
206 206
 }
207 207
 
208
-AmSession* AmSessionContainer::startSessionUAC(AmSipRequest& req, AmArg* session_params) {
208
+string AmSessionContainer::startSessionUAC(AmSipRequest& req, AmArg* session_params) {
209 209
 
210
-  AmSession* session = NULL;
210
+  auto_ptr<AmSession> session;
211 211
   try {
212
-    if((session = createSession(req, session_params)) != 0) {
212
+    session.reset(createSession(req, session_params));
213
+    if(session.get() != 0) {
214
+
213 215
       session->dlg.updateStatusFromLocalRequest(req);
214 216
       session->setCallgroup(req.from_tag);
215 217
 
216 218
       session->setNegotiateOnReply(true);
217 219
 
218
-      if (!addSession("","",req.from_tag,session)) {
220
+      switch(addSession(req.from_tag,session.get())) {
221
+	
222
+      case AmSessionContainer::Inserted:
223
+	// successful case
224
+	break;
225
+	
226
+      case AmSessionContainer::ShutDown:
227
+	throw AmSession::Exception(AmConfig::ShutdownModeErrCode,
228
+				   AmConfig::ShutdownModeErrReason);
229
+	
230
+      case AmSessionContainer::AlreadyExist:
231
+	throw AmSession::Exception(482,
232
+				   SIP_REPLY_LOOP_DETECTED);
233
+	
234
+      default:
219 235
 	ERROR("adding session to session container\n");
220
-	delete session;
221
-	return NULL;
236
+	throw string(SIP_REPLY_SERVER_INTERNAL_ERROR);
222 237
       }
223
-
224
-      MONITORING_LOG5(session->getLocalTag().c_str(), 
238
+      
239
+      MONITORING_LOG5(req.from_tag, 
225 240
 		      "app", req.cmd.c_str(),
226 241
 		      "dir", "out",
227 242
 		      "from", req.from.c_str(),
... ...
@@ -229,59 +244,49 @@ AmSession* AmSessionContainer::startSessionUAC(AmSipRequest& req, AmArg* session
229 244
 		      "ruri", req.r_uri.c_str());
230 245
       
231 246
       if (int err = session->sendInvite(req.hdrs)) {
232
-	ERROR("INVITE could not be sent: error code = %d.\n", 
233
-	      err);
234
-	AmEventDispatcher::instance()->
235
-	  delEventQueue(session->getLocalTag(),
236
-			session->getCallID(),
237
-			session->getRemoteTag());	
238
-	MONITORING_MARK_FINISHED(session->getLocalTag().c_str());
239
-	delete session;
247
+	ERROR("INVITE could not be sent: error code = %d.\n", err);
248
+	AmEventDispatcher::instance()->delEventQueue(req.from_tag);
249
+	MONITORING_MARK_FINISHED(req.from_tag.c_str());
240 250
 	return NULL;
241 251
       }
242
-
252
+      
243 253
       if (AmConfig::LogSessions) {      
244 254
 	INFO("Starting UAC session %s app %s\n",
245
-	     session->getLocalTag().c_str(), req.cmd.c_str());
255
+	     req.from_tag.c_str(), req.cmd.c_str());
246 256
       }
247
-      
248 257
       try {
249 258
 	session->start();
250 259
       } catch (...) {
251
-	AmEventDispatcher::instance()->
252
-	  delEventQueue(session->getLocalTag(),
253
-			session->getCallID(),
254
-			session->getRemoteTag());
255
-
256
-	delete session;
257
-	session = NULL;
260
+	AmEventDispatcher::instance()->delEventQueue(req.from_tag);
258 261
 	throw;
259 262
       }
260
-
261 263
     }
262 264
   } 
263 265
   catch(const AmSession::Exception& e){
264 266
     ERROR("%i %s\n",e.code,e.reason.c_str());
265
-    AmSipDialog::reply_error(req,e.code,e.reason);
267
+    return "";
266 268
   }
267 269
   catch(const string& err){
268 270
     ERROR("startSession: %s\n",err.c_str());
269
-    AmSipDialog::reply_error(req,500,err);
271
+    return "";
270 272
   }
271 273
   catch(...){
272 274
     ERROR("unexpected exception\n");
273
-    AmSipDialog::reply_error(req,500,"unexpected exception");
275
+    return "";
274 276
   }
275 277
 
276
-  return session;
278
+  session.release();
279
+  return req.from_tag;
277 280
 }
278 281
 
279 282
 void AmSessionContainer::startSessionUAS(AmSipRequest& req)
280 283
 {
281 284
   try {
282 285
       // Call-ID and From-Tag are unknown: it's a new session
283
-      AmSession* session;
284
-      if((session = createSession(req)) != 0){
286
+      auto_ptr<AmSession> session;
287
+
288
+      session.reset(createSession(req));
289
+      if(session.get() != 0){
285 290
 
286 291
 	// update session's local tag (ID) if not already set
287 292
 	session->setLocalTag();
... ...
@@ -294,9 +299,23 @@ void AmSessionContainer::startSessionUAS(AmSipRequest& req)
294 299
 	       local_tag.c_str(), req.cmd.c_str());
295 300
 	}
296 301
 
297
-	if (!addSession(req.callid,req.from_tag,local_tag,session)) {
302
+	switch(addSession(req.callid,req.from_tag,local_tag,
303
+			  session.get())) {
304
+
305
+	case AmSessionContainer::Inserted:
306
+	  // successful case
307
+	  break;
308
+	  
309
+	case AmSessionContainer::ShutDown:
310
+	  throw AmSession::Exception(AmConfig::ShutdownModeErrCode,
311
+				     AmConfig::ShutdownModeErrReason);
312
+	  
313
+	case AmSessionContainer::AlreadyExist:
314
+	  throw AmSession::Exception(482,
315
+				     SIP_REPLY_LOOP_DETECTED);
316
+	  
317
+	default:
298 318
 	  ERROR("adding session to session container\n");
299
-	  delete session;
300 319
 	  throw string(SIP_REPLY_SERVER_INTERNAL_ERROR);
301 320
 	}
302 321
 
... ...
@@ -309,17 +328,15 @@ void AmSessionContainer::startSessionUAS(AmSipRequest& req)
309 328
 
310 329
 	try {
311 330
 	  session->start();
312
-	} catch (const string& err) {
331
+	  session.release();
332
+	} catch (...) {
313 333
 	  AmEventDispatcher::instance()->
314
-	    delEventQueue(session->getLocalTag(),
315
-			  session->getCallID(),
316
-			  session->getRemoteTag());
317
-	  
318
-	  delete session;
334
+	    delEventQueue(req.callid,req.from_tag,local_tag);
319 335
 	  throw;
320 336
 	}
321 337
 
322 338
 	session->postEvent(new AmSipRequestEvent(req));
339
+	session.release();
323 340
       }
324 341
   } 
325 342
   catch(const AmSession::Exception& e){
... ...
@@ -426,27 +443,37 @@ AmSession* AmSessionContainer::createSession(AmSipRequest& req,
426 443
   return session;
427 444
 }
428 445
 
429
-bool AmSessionContainer::addSession(const string& callid,
430
-				    const string& remote_tag,
431
-				    const string& local_tag,
432
-				    AmSession* session)
446
+AmSessionContainer::AddSessionStatus 
447
+AmSessionContainer::addSession(const string& callid,
448
+			       const string& remote_tag,
449
+			       const string& local_tag,
450
+			       AmSession* session)
433 451
 {
434 452
   if(_container_closed.get())
435
-    return false;
453
+    return ShutDown;
436 454
   
437
-  return AmEventDispatcher::instance()->
438
-    addEventQueue(local_tag,(AmEventQueue*)session,
439
-		  callid,remote_tag);
455
+  if(AmEventDispatcher::instance()->
456
+     addEventQueue(local_tag,(AmEventQueue*)session,
457
+		   callid,remote_tag)) {
458
+    return Inserted;
459
+  }
460
+
461
+  return AlreadyExist;
440 462
 }
441 463
 
442
-bool AmSessionContainer::addSession(const string& local_tag,
443
-				    AmSession* session)
464
+AmSessionContainer::AddSessionStatus 
465
+AmSessionContainer::addSession(const string& local_tag,
466
+			       AmSession* session)
444 467
 {
445 468
   if(_container_closed.get()) 
446
-    return false;
469
+    return ShutDown;
470
+
471
+  if(AmEventDispatcher::instance()->
472
+     addEventQueue(local_tag,(AmEventQueue*)session)){
473
+    return Inserted;
474
+  }
447 475
 
448
-  return AmEventDispatcher::instance()->
449
-    addEventQueue(local_tag,(AmEventQueue*)session);
476
+  return AlreadyExist;
450 477
 }
451 478
 
452 479
 void AmSessionContainer::enableUncleanShutdown() {
... ...
@@ -87,6 +87,12 @@ class AmSessionContainer : public AmThread
87 87
 
88 88
   static void dispose();
89 89
 
90
+  enum AddSessionStatus {
91
+    ShutDown,
92
+    Inserted,
93
+    AlreadyExist
94
+  };
95
+
90 96
   /**
91 97
    * Creates a new session.
92 98
    * @param req local request
... ...
@@ -99,17 +105,17 @@ class AmSessionContainer : public AmThread
99 105
    * Adds a session to the container (UAS only).
100 106
    * @return true if the session is new within the container.
101 107
    */
102
-  bool addSession(const string& callid,
103
-		  const string& remote_tag,
104
-		  const string& local_tag,
105
-		  AmSession* session);
108
+  AddSessionStatus addSession(const string& callid,
109
+			      const string& remote_tag,
110
+			      const string& local_tag,
111
+			      AmSession* session);
106 112
 
107 113
   /**
108 114
    * Adds a session to the container.
109 115
    * @return true if the session is new within the container.
110 116
    */
111
-  bool addSession(const string& local_tag,
112
- 		  AmSession* session);
117
+  AddSessionStatus addSession(const string& local_tag,
118
+			      AmSession* session);
113 119
 
114 120
   /** 
115 121
    * Constructs a new session and adds it to the active session container. 
... ...
@@ -121,8 +127,8 @@ class AmSessionContainer : public AmThread
121 127
    * Constructs a new session and adds it to the active session container. 
122 128
    * @param req client's request
123 129
    */
124
-  AmSession* startSessionUAC(AmSipRequest& req, 
125
-			     AmArg* session_params = NULL);
130
+  string startSessionUAC(AmSipRequest& req, 
131
+			 AmArg* session_params = NULL);
126 132
 
127 133
   /**
128 134
    * Detroys a session.
... ...
@@ -87,7 +87,20 @@ void AmSipDialog::updateStatus(const AmSipRequest& req)
87 87
 {
88 88
   DBG("AmSipDialog::updateStatus(req = %s)\n", req.method.c_str());
89 89
 
90
-  if ((req.method == "ACK") || (req.method == "CANCEL")) {
90
+  if (req.method == "ACK") {
91
+    if(hdl)
92
+      hdl->onSipRequest(req);
93
+    return;
94
+  }
95
+
96
+  if (req.method == "CANCEL") {
97
+
98
+    TransMap::iterator t_it = uas_trans.find(req.cseq);
99
+    if(t_it == uas_trans.end()){
100
+      reply_error(req,481,SIP_REPLY_NOT_EXIST);
101
+      return;
102
+    }
103
+
91 104
     if(hdl)
92 105
       hdl->onSipRequest(req);
93 106
     return;
... ...
@@ -236,8 +249,9 @@ int AmSipDialog::updateStatusReply(const AmSipRequest& req, unsigned int code)
236 249
   TransMap::iterator t_it = uas_trans.find(req.cseq);
237 250
   if(t_it == uas_trans.end()){
238 251
     ERROR("could not find any transaction matching request\n");
239
-    ERROR("method=%s; callid=%s; local_tag=%s; remote_tag=%s; cseq=%i\n",
240
-	  req.method.c_str(),callid.c_str(),local_tag.c_str(),
252
+    ERROR("reply code=%i; method=%s; callid=%s; local_tag=%s; "
253
+	  "remote_tag=%s; cseq=%i\n",
254
+	  code,req.method.c_str(),callid.c_str(),local_tag.c_str(),
241 255
 	  remote_tag.c_str(),req.cseq);
242 256
     return -1;
243 257
   }
... ...
@@ -53,4 +53,7 @@
53 53
 #define SIP_REPLY_SERVER_INTERNAL_ERROR "Server Internal Error"
54 54
 #define SIP_REPLY_BAD_EXTENSION         "Bad Extension"
55 55
 #define SIP_REPLY_EXTENSION_REQUIRED    "Extension Required"
56
+#define SIP_REPLY_LOOP_DETECTED         "Loop Detected"
57
+#define SIP_REPLY_NOT_EXIST             "Call Leg/Transaction Does Not Exist"
58
+
56 59
 #endif /* __AMSIPHEADERS_H__ */
... ...
@@ -31,15 +31,15 @@
31 31
 #include "AmSessionContainer.h"
32 32
 #include "AmConfig.h"
33 33
 
34
-AmSession* AmUAC::dialout(const string& user,
35
-			  const string& app_name,
36
-			  const string& r_uri, 
37
-			  const string& from,
38
-			  const string& from_uri,
39
-			  const string& to,
40
-			  const string& local_tag,
41
-			  const string& hdrs,
42
-			  AmArg*  session_params) {
34
+string AmUAC::dialout(const string& user,
35
+		      const string& app_name,
36
+		      const string& r_uri, 
37
+		      const string& from,
38
+		      const string& from_uri,
39
+		      const string& to,
40
+		      const string& local_tag,
41
+		      const string& hdrs,
42
+		      AmArg*  session_params) {
43 43
  
44 44
   AmSipRequest req;
45 45
 
... ...
@@ -38,15 +38,15 @@ using std::string;
38 38
 /** \brief API for UAC support */
39 39
 class AmUAC {
40 40
  public:
41
-  static AmSession* dialout(const string& user,
42
-			    const string& app_name,
43
-			    const string& r_uri, 
44
-			    const string& from,
45
-			    const string& from_uri,
46
-			    const string& to,
47
-			    const string& local_tag = "",
48
-			    const string& hdrs = "",
49
-			    AmArg*  session_params = NULL);
41
+  static string dialout(const string& user,
42
+			const string& app_name,
43
+			const string& r_uri, 
44
+			const string& from,
45
+			const string& from_uri,
46
+			const string& to,
47
+			const string& local_tag = "",
48
+			const string& hdrs = "",
49
+			AmArg*  session_params = NULL);
50 50
 
51 51
 };
52 52