Ignore:
Timestamp:
Feb 21, 2013 11:26:35 AM (11 years ago)
Author:
bennylp
Message:

Fixed #1617: major synchronization fixes in PJNATH with incorporation of group lock to avoid deadlock and crashes due to race conditions

File:
1 edited

Legend:

Unmodified
Added
Removed

  • pjproject/trunk/pjnath/src/pjnath/stun_sock.c

    r4344 r4360  
    2929#include <pj/ip_helper.h> 
    3030#include <pj/log.h> 
     31#include <pj/os.h> 
    3132#include <pj/pool.h> 
    3233#include <pj/rand.h> 
    3334 
     35#if 1 
     36#  define TRACE_(x)     PJ_LOG(5,x) 
     37#else 
     38#  define TRACE_(x) 
     39#endif 
    3440 
    3541enum { MAX_BIND_RETRY = 100 }; 
    … …  
    4046    pj_pool_t           *pool;          /* Pool                     */ 
    4147    void                *user_data;     /* Application user data    */ 
    42  
     48    pj_bool_t            is_destroying; /* Destroy already called   */ 
    4349    int                  af;            /* Address family           */ 
    4450    pj_stun_config       stun_cfg;      /* STUN config (ioqueue etc)*/ 
    … …  
    5965    pj_uint16_t          tsx_id[6];     /* .. to match STUN msg     */ 
    6066    pj_stun_session     *stun_sess;     /* STUN session             */ 
    61  
     67    pj_grp_lock_t       *grp_lock;      /* Session group lock       */ 
    6268}; 
    6369 
    … …  
    6571 * Prototypes for static functions  
    6672 */ 
     73 
     74/* Destructor for group lock */ 
     75static void stun_sock_destructor(void *obj); 
    6776 
    6877/* This callback is called by the STUN session to send packet */ 
    … …  
    203212        stun_sock->ka_interval = PJ_STUN_KEEP_ALIVE_SEC; 
    204213 
     214    if (cfg && cfg->grp_lock) { 
     215        stun_sock->grp_lock = cfg->grp_lock; 
     216    } else { 
     217        status = pj_grp_lock_create(pool, NULL, &stun_sock->grp_lock); 
     218        if (status != PJ_SUCCESS) { 
     219            pj_pool_release(pool); 
     220            return status; 
     221        } 
     222    } 
     223 
     224    pj_grp_lock_add_ref(stun_sock->grp_lock); 
     225    pj_grp_lock_add_handler(stun_sock->grp_lock, pool, stun_sock, 
     226                            &stun_sock_destructor); 
     227 
    205228    /* Create socket and bind socket */ 
    206229    status = pj_sock_socket(af, pj_SOCK_DGRAM(), 0, &stun_sock->sock_fd); 
    … …  
    253276 
    254277        pj_activesock_cfg_default(&activesock_cfg); 
     278        activesock_cfg.grp_lock = stun_sock->grp_lock; 
    255279        activesock_cfg.async_cnt = cfg->async_cnt; 
    256280        activesock_cfg.concurrency = 0; 
    … …  
    291315                                        stun_sock->obj_name, 
    292316                                        &sess_cb, PJ_FALSE,  
     317                                        stun_sock->grp_lock, 
    293318                                        &stun_sock->stun_sess); 
    294319        if (status != PJ_SUCCESS) 
    … …  
    333358    PJ_ASSERT_RETURN(stun_sock && domain && default_port, PJ_EINVAL); 
    334359 
     360    pj_grp_lock_acquire(stun_sock->grp_lock); 
     361 
    335362    /* Check whether the domain contains IP address */ 
    336363    stun_sock->srv_addr.addr.sa_family = (pj_uint16_t)stun_sock->af; 
    … …  
    361388 
    362389        /* Processing will resume when the DNS SRV callback is called */ 
    363         return status; 
    364390 
    365391    } else { 
    … …  
    379405 
    380406        /* Start sending Binding request */ 
    381         return get_mapped_addr(stun_sock); 
    382     } 
    383 } 
    384  
    385 /* Destroy */ 
    386 PJ_DEF(pj_status_t) pj_stun_sock_destroy(pj_stun_sock *stun_sock) 
    387 { 
     407        status = get_mapped_addr(stun_sock); 
     408    } 
     409 
     410    pj_grp_lock_release(stun_sock->grp_lock); 
     411    return status; 
     412} 
     413 
     414/* Destructor */ 
     415static void stun_sock_destructor(void *obj) 
     416{ 
     417    pj_stun_sock *stun_sock = (pj_stun_sock*)obj; 
     418 
    388419    if (stun_sock->q) { 
    389420        pj_dns_srv_cancel_query(stun_sock->q, PJ_FALSE); 
    … …  
    391422    } 
    392423 
    393     if (stun_sock->stun_sess) { 
    394         pj_stun_session_set_user_data(stun_sock->stun_sess, NULL); 
    395     } 
    396      
    397     /* Destroy the active socket first just in case we'll get 
    398      * stray callback. 
    399      */ 
    400     if (stun_sock->active_sock != NULL) { 
    401         pj_activesock_t *asock = stun_sock->active_sock; 
    402         stun_sock->active_sock = NULL; 
    403         stun_sock->sock_fd = PJ_INVALID_SOCKET; 
    404         pj_activesock_set_user_data(asock, NULL); 
    405         pj_activesock_close(asock); 
    406     } else if (stun_sock->sock_fd != PJ_INVALID_SOCKET) { 
    407         pj_sock_close(stun_sock->sock_fd); 
    408         stun_sock->sock_fd = PJ_INVALID_SOCKET; 
    409     } 
    410  
    411     if (stun_sock->ka_timer.id != 0) { 
    412         pj_timer_heap_cancel(stun_sock->stun_cfg.timer_heap,  
    413                              &stun_sock->ka_timer); 
    414         stun_sock->ka_timer.id = 0; 
    415     } 
    416  
     424    /* 
    417425    if (stun_sock->stun_sess) { 
    418426        pj_stun_session_destroy(stun_sock->stun_sess); 
    419427        stun_sock->stun_sess = NULL; 
    420428    } 
     429    */ 
    421430 
    422431    if (stun_sock->pool) { 
    … …  
    426435    } 
    427436 
     437    TRACE_(("", "STUN sock %p destroyed", stun_sock)); 
     438 
     439} 
     440 
     441/* Destroy */ 
     442PJ_DEF(pj_status_t) pj_stun_sock_destroy(pj_stun_sock *stun_sock) 
     443{ 
     444    TRACE_((stun_sock->obj_name, "STUN sock %p request, ref_cnt=%d", 
     445            stun_sock, pj_grp_lock_get_ref(stun_sock->grp_lock))); 
     446 
     447    pj_grp_lock_acquire(stun_sock->grp_lock); 
     448    if (stun_sock->is_destroying) { 
     449        /* Destroy already called */ 
     450        pj_grp_lock_release(stun_sock->grp_lock); 
     451        return PJ_EINVALIDOP; 
     452    } 
     453 
     454    stun_sock->is_destroying = PJ_TRUE; 
     455    pj_timer_heap_cancel_if_active(stun_sock->stun_cfg.timer_heap, 
     456                                   &stun_sock->ka_timer, 0); 
     457 
     458    if (stun_sock->active_sock != NULL) { 
     459        stun_sock->sock_fd = PJ_INVALID_SOCKET; 
     460        pj_activesock_close(stun_sock->active_sock); 
     461    } else if (stun_sock->sock_fd != PJ_INVALID_SOCKET) { 
     462        pj_sock_close(stun_sock->sock_fd); 
     463        stun_sock->sock_fd = PJ_INVALID_SOCKET; 
     464    } 
     465 
     466    if (stun_sock->stun_sess) { 
     467        pj_stun_session_destroy(stun_sock->stun_sess); 
     468    } 
     469    pj_grp_lock_dec_ref(stun_sock->grp_lock); 
     470    pj_grp_lock_release(stun_sock->grp_lock); 
    428471    return PJ_SUCCESS; 
    429472} 
    … …  
    469512    pj_stun_sock *stun_sock = (pj_stun_sock*) user_data; 
    470513 
     514    pj_grp_lock_acquire(stun_sock->grp_lock); 
     515 
    471516    /* Clear query */ 
    472517    stun_sock->q = NULL; 
    … …  
    475520    if (status != PJ_SUCCESS) { 
    476521        sess_fail(stun_sock, PJ_STUN_SOCK_DNS_OP, status); 
     522        pj_grp_lock_release(stun_sock->grp_lock); 
    477523        return; 
    478524    } 
    … …  
    491537    /* Start sending Binding request */ 
    492538    get_mapped_addr(stun_sock); 
     539 
     540    pj_grp_lock_release(stun_sock->grp_lock); 
    493541} 
    494542 
    … …  
    534582    PJ_ASSERT_RETURN(stun_sock && info, PJ_EINVAL); 
    535583 
     584    pj_grp_lock_acquire(stun_sock->grp_lock); 
     585 
    536586    /* Copy STUN server address and mapped address */ 
    537587    pj_memcpy(&info->srv_addr, &stun_sock->srv_addr, 
    … …  
    544594    status = pj_sock_getsockname(stun_sock->sock_fd, &info->bound_addr, 
    545595                                 &addr_len); 
    546     if (status != PJ_SUCCESS) 
     596    if (status != PJ_SUCCESS) { 
     597        pj_grp_lock_release(stun_sock->grp_lock); 
    547598        return status; 
     599    } 
    548600 
    549601    /* If socket is bound to a specific interface, then only put that 
    … …  
    561613        /* Get the default address */ 
    562614        status = pj_gethostip(stun_sock->af, &def_addr); 
    563         if (status != PJ_SUCCESS) 
     615        if (status != PJ_SUCCESS) { 
     616            pj_grp_lock_release(stun_sock->grp_lock); 
    564617            return status; 
     618        } 
    565619         
    566620        pj_sockaddr_set_port(&def_addr, port); 
    … …  
    570624        status = pj_enum_ip_interface(stun_sock->af, &info->alias_cnt,  
    571625                                      info->aliases); 
    572         if (status != PJ_SUCCESS) 
     626        if (status != PJ_SUCCESS) { 
     627            pj_grp_lock_release(stun_sock->grp_lock); 
    573628            return status; 
     629        } 
    574630 
    575631        /* Set the port number for each address. 
    … …  
    591647    } 
    592648 
     649    pj_grp_lock_release(stun_sock->grp_lock); 
    593650    return PJ_SUCCESS; 
    594651} 
    … …  
    604661{ 
    605662    pj_ssize_t size; 
     663    pj_status_t status; 
     664 
    606665    PJ_ASSERT_RETURN(stun_sock && pkt && dst_addr && addr_len, PJ_EINVAL); 
    607666     
     667    pj_grp_lock_acquire(stun_sock->grp_lock); 
     668 
     669    if (!stun_sock->active_sock) { 
     670        /* We have been shutdown, but this callback may still get called 
     671         * by retransmit timer. 
     672         */ 
     673        pj_grp_lock_release(stun_sock->grp_lock); 
     674        return PJ_EINVALIDOP; 
     675    } 
     676 
    608677    if (send_key==NULL) 
    609678        send_key = &stun_sock->send_key; 
    610679 
    611680    size = pkt_len; 
    612     return pj_activesock_sendto(stun_sock->active_sock, send_key,  
    613                                 pkt, &size, flag, dst_addr, addr_len); 
     681    status = pj_activesock_sendto(stun_sock->active_sock, send_key, 
     682                                  pkt, &size, flag, dst_addr, addr_len); 
     683 
     684    pj_grp_lock_release(stun_sock->grp_lock); 
     685    return status; 
    614686} 
    615687 
    … …  
    626698 
    627699    stun_sock = (pj_stun_sock *) pj_stun_session_get_user_data(sess); 
    628     if (!stun_sock || !stun_sock->active_sock) 
     700    if (!stun_sock || !stun_sock->active_sock) { 
     701        /* We have been shutdown, but this callback may still get called 
     702         * by retransmit timer. 
     703         */ 
    629704        return PJ_EINVALIDOP; 
     705    } 
    630706 
    631707    pj_assert(token==INTERNAL_MSG_TOKEN); 
    … …  
    633709 
    634710    size = pkt_size; 
    635     return pj_activesock_sendto(stun_sock->active_sock,  
     711    return pj_activesock_sendto(stun_sock->active_sock, 
    636712                                &stun_sock->int_send_key, 
    637713                                pkt, &size, 0, dst_addr, addr_len); 
    … …  
    727803static void start_ka_timer(pj_stun_sock *stun_sock) 
    728804{ 
    729     if (stun_sock->ka_timer.id != 0) { 
    730         pj_timer_heap_cancel(stun_sock->stun_cfg.timer_heap,  
    731                              &stun_sock->ka_timer); 
    732         stun_sock->ka_timer.id = 0; 
    733     } 
     805    pj_timer_heap_cancel_if_active(stun_sock->stun_cfg.timer_heap, 
     806                                   &stun_sock->ka_timer, 0); 
    734807 
    735808    pj_assert(stun_sock->ka_interval != 0); 
    736     if (stun_sock->ka_interval > 0) { 
     809    if (stun_sock->ka_interval > 0 && !stun_sock->is_destroying) { 
    737810        pj_time_val delay; 
    738811 
    … …  
    740813        delay.msec = 0; 
    741814 
    742         if (pj_timer_heap_schedule(stun_sock->stun_cfg.timer_heap,  
    743                                    &stun_sock->ka_timer,  
    744                                    &delay) == PJ_SUCCESS) 
    745         { 
    746             stun_sock->ka_timer.id = PJ_TRUE; 
    747         } 
     815        pj_timer_heap_schedule_w_grp_lock(stun_sock->stun_cfg.timer_heap, 
     816                                          &stun_sock->ka_timer, 
     817                                          &delay, PJ_TRUE, 
     818                                          stun_sock->grp_lock); 
    748819    } 
    749820} 
    … …  
    757828 
    758829    PJ_UNUSED_ARG(th); 
     830    pj_grp_lock_acquire(stun_sock->grp_lock); 
    759831 
    760832    /* Time to send STUN Binding request */ 
    761     if (get_mapped_addr(stun_sock) != PJ_SUCCESS) 
     833    if (get_mapped_addr(stun_sock) != PJ_SUCCESS) { 
     834        pj_grp_lock_release(stun_sock->grp_lock); 
    762835        return; 
     836    } 
    763837 
    764838    /* Next keep-alive timer will be scheduled once the request 
    765839     * is complete. 
    766840     */ 
     841    pj_grp_lock_release(stun_sock->grp_lock); 
    767842} 
    768843 
    … …  
    788863        return PJ_TRUE; 
    789864    } 
     865 
     866    pj_grp_lock_acquire(stun_sock->grp_lock); 
    790867 
    791868    /* Check that this is STUN message */ 
    … …  
    824901                                       PJ_STUN_IS_DATAGRAM, NULL, NULL, 
    825902                                       src_addr, addr_len); 
    826     return status!=PJNATH_ESTUNDESTROYED ? PJ_TRUE : PJ_FALSE; 
     903 
     904    status = pj_grp_lock_release(stun_sock->grp_lock); 
     905 
     906    return status!=PJ_EGONE ? PJ_TRUE : PJ_FALSE; 
    827907 
    828908process_app_data: 
    … …  
    832912        ret = (*stun_sock->cb.on_rx_data)(stun_sock, data, size, 
    833913                                          src_addr, addr_len); 
    834         return ret; 
    835     } 
    836  
    837     return PJ_TRUE; 
     914        status = pj_grp_lock_release(stun_sock->grp_lock); 
     915        return status!=PJ_EGONE ? PJ_TRUE : PJ_FALSE; 
     916    } 
     917 
     918    status = pj_grp_lock_release(stun_sock->grp_lock); 
     919    return status!=PJ_EGONE ? PJ_TRUE : PJ_FALSE; 
    838920} 
    839921 
    … …  
    857939    if (stun_sock->cb.on_data_sent) { 
    858940        pj_bool_t ret; 
     941 
     942        pj_grp_lock_acquire(stun_sock->grp_lock); 
    859943 
    860944        /* If app gives NULL send_key in sendto() function, then give 
    … …  
    867951        ret = (*stun_sock->cb.on_data_sent)(stun_sock, send_key, sent); 
    868952 
     953        pj_grp_lock_release(stun_sock->grp_lock); 
    869954        return ret; 
    870955    } 
Note: See TracChangeset for help on using the changeset viewer.