pgaval / unimrcp

Automatically exported from code.google.com/p/unimrcp
Apache License 2.0
0 stars 0 forks source link

using occupied SIP origin port fails silently, blocks client #127

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. launch an MRCP client with a specific SIP port, start a session.
2. launch another client with the same port configuration.
3. launch an MRCP session/request on the second client.
4. second client fails silently (SDP offer never gets sent but doesn't cause 
fail), becomes blocked without clear error indication except perhaps timeout 
(depending on client code).

I know that using the same origin port is a misconfiguration and can not be 
done. But this is still an important issue because the software product I'm 
working on is very large, very complex, and this kind of misconfiguration is 
quite possible in production environments.

So, a very clear failure status is highly desirable in this use case.
I believe unimrcp should immediately return a clear error when any requests are 
performed while the SIP stack is not ready (for any reason).

The problem starts when the call to 
unimrcp_client_create()
 ...
  mrcp_sofiasip_client_agent_create()
    [another thread]
      mrcp_sofia_task_initialize()
        nua_create()
          [another thread]
            nua_stack_init()
              nua_stack_init_transport()
                nta_agent_add_tport()
                  tport_tbind()
fails because the port is already taken. This is done via 2 asynchronous tasks, 
so no return error is reported. 

Next, because the client application did not get an error, it goes on to create 
a channel:
mrcp_application_channel_add()
  mrcp_app_signaling_task_msg_signal()
    [another thread]
      mrcp_app_request_dispatch()
        mrcp_sofia_session_create()
          nua_handle()
The mrcp_sofia_session_create() function fails silently, always returns True, 
and the "sofia_session->nh" handle is not created and stays null. But the 
unimrcp code goes on to call all those resource allocators anyway, as if there 
actually was a connection:
mrcp_client_channel_add()
  mrcp_session_descriptor_create()
  mrcp_client_control_channel_create()
  mrcp_control_offer_create()
  mrcp_session_control_media_add()
  mrcp_cmid_add()
  mrcp_client_control_channel_add()
  mrcp_client_session_subrequest_add()
  mpf_engine_context_create()
  mpf_engine_termination_message_add()
  mrcp_client_session_subrequest_add()
  mpf_rtp_termination_descriptor_init()
  mrcp_client_session_offer_send()
  mpf_rtp_media_descriptor_init()
  mpf_termination_create()
  mpf_engine_termination_message_add()
  mrcp_client_session_subrequest_add()
  mpf_engine_message_send()
    [another thread]
      mrcp_client_on_termination_add()
        mrcp_client_session_offer_send()
          mrcp_session_offer()
            mrcp_sofia_session_offer()
              nua_invite()
The nua_invite never actually takes place because there is no valid 
sofia_session->nh, and the error is returned all the way back to 
mrcp_client_on_termination_add(), which then discards it.

The next thing that happens is that the client app will only be capable of 
detecting the error via time-out, which is less than desirable since the error 
is local to the client machine and could be detected immediately.

At this point, I'm not quite sure how I can do 2 things:
1 - check that the SIP stack is actually functional;
2 - abort the created mrcp_session cleanly, releasing all resources from all 
levels immediately, when the SIP connection is not available.

I've tried calling mrcp_session_destroy() when the time-out occurs, but I'm 
worried this is not a clean way of doing it because 
mrcp_client_session_t*->active_request is not null.

Versions tested: 
-unimcrp: r1798
-sofiasip: 1.12.10

Original issue reported on code.google.com by vasco.ne...@sapo.pt on 27 Jul 2011 at 4:55

GoogleCodeExporter commented 9 years ago
I'm testing a patch.
Tell me what you think about this.

modules\mrcp-sofiasip\src\mrcp_sofiasip_client_agent.c

93a94,101
> static void mrcp_sofia_on_session_terminate(
>                                               int                   status,
>                                               mrcp_sofia_agent_t   
*sofia_agent,
>                                               nua_handle_t         *nh,
>                                               mrcp_sofia_session_t 
*sofia_session,
>                                               sip_t const          *sip,
>                                               tagi_t                tags[]);
>
329,331c338,340
<       apr_thread_mutex_lock(sofia_session->mutex);
<
<       if(sofia_session->nh) {
---
>       if(sofia_session->nh)
>     {
>       apr_thread_mutex_lock(sofia_session->mutex);
336,338c345,352
<       }
<
<       apr_thread_mutex_unlock(sofia_session->mutex);
---
>       apr_thread_mutex_unlock(sofia_session->mutex);
>       }
>     else
>     {
>               apt_log(APT_LOG_MARK,APT_PRIO_INFO,"SIP stack is invalid. 
Destroying session."APT_NAMESID_FMT"\n", session->name, 
MRCP_SESSION_SID(session));
>         mrcp_sofia_on_session_terminate(0, NULL, NULL, sofia_session, NULL, 
NULL);
>     }
>

Original comment by vasco.ne...@sapo.pt on 16 Aug 2011 at 9:54

GoogleCodeExporter commented 9 years ago
Fixed in r1894.

Original comment by achalo...@gmail.com on 7 Mar 2013 at 2:45