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/ice_strans.c

    r4314 r4360  
    127127 
    128128/* Forward decls */ 
     129static void ice_st_on_destroy(void *obj); 
    129130static void destroy_ice_st(pj_ice_strans *ice_st); 
    130131#define ice_st_perror(ice_st,msg,rc) pjnath_perror(ice_st->obj_name,msg,rc) 
    131132static void sess_init_update(pj_ice_strans *ice_st); 
    132  
    133 static void sess_add_ref(pj_ice_strans *ice_st); 
    134 static pj_bool_t sess_dec_ref(pj_ice_strans *ice_st); 
    135133 
    136134/** 
    … …  
    173171    pj_ice_strans_cfg        cfg;       /**< Configuration.             */ 
    174172    pj_ice_strans_cb         cb;        /**< Application callback.      */ 
    175     pj_lock_t               *init_lock; /**< Initialization mutex.      */ 
     173    pj_grp_lock_t           *grp_lock;  /**< Group lock.                */ 
    176174 
    177175    pj_ice_strans_state      state;     /**< Session state.             */ 
    … …  
    184182    pj_timer_entry           ka_timer;  /**< STUN keep-alive timer.     */ 
    185183 
    186     pj_atomic_t             *busy_cnt;  /**< To prevent destroy         */ 
    187184    pj_bool_t                destroy_req;/**< Destroy has been called?  */ 
    188185    pj_bool_t                cb_called; /**< Init error callback called?*/ 
    … …  
    552549    pj_log_push_indent(); 
    553550 
    554     pj_ice_strans_cfg_copy(pool, &ice_st->cfg, cfg); 
    555     pj_memcpy(&ice_st->cb, cb, sizeof(*cb)); 
    556      
    557     status = pj_atomic_create(pool, 0, &ice_st->busy_cnt); 
     551    status = pj_grp_lock_create(pool, NULL, &ice_st->grp_lock); 
    558552    if (status != PJ_SUCCESS) { 
    559         destroy_ice_st(ice_st); 
    560         return status; 
    561     } 
    562  
    563     status = pj_lock_create_recursive_mutex(pool, ice_st->obj_name,  
    564                                             &ice_st->init_lock); 
    565     if (status != PJ_SUCCESS) { 
    566         destroy_ice_st(ice_st); 
     553        pj_pool_release(pool); 
    567554        pj_log_pop_indent(); 
    568555        return status; 
    569556    } 
     557 
     558    pj_grp_lock_add_ref(ice_st->grp_lock); 
     559    pj_grp_lock_add_handler(ice_st->grp_lock, pool, ice_st, 
     560                            &ice_st_on_destroy); 
     561 
     562    pj_ice_strans_cfg_copy(pool, &ice_st->cfg, cfg); 
     563    ice_st->cfg.stun.cfg.grp_lock = ice_st->grp_lock; 
     564    ice_st->cfg.turn.cfg.grp_lock = ice_st->grp_lock; 
     565    pj_memcpy(&ice_st->cb, cb, sizeof(*cb)); 
    570566 
    571567    ice_st->comp_cnt = comp_cnt; 
    … …  
    579575     * called before we finish initialization. 
    580576     */ 
    581     pj_lock_acquire(ice_st->init_lock); 
     577    pj_grp_lock_acquire(ice_st->grp_lock); 
    582578 
    583579    for (i=0; i<comp_cnt; ++i) { 
    584580        status = create_comp(ice_st, i+1); 
    585581        if (status != PJ_SUCCESS) { 
    586             pj_lock_release(ice_st->init_lock); 
     582            pj_grp_lock_release(ice_st->grp_lock); 
    587583            destroy_ice_st(ice_st); 
    588584            pj_log_pop_indent(); 
    … …  
    592588 
    593589    /* Done with initialization */ 
    594     pj_lock_release(ice_st->init_lock); 
    595  
    596     PJ_LOG(4,(ice_st->obj_name, "ICE stream transport created")); 
     590    pj_grp_lock_release(ice_st->grp_lock); 
     591 
     592    PJ_LOG(4,(ice_st->obj_name, "ICE stream transport %p created", ice_st)); 
    597593 
    598594    *p_ice_st = ice_st; 
    … …  
    606602} 
    607603 
     604/* REALLY destroy ICE */ 
     605static void ice_st_on_destroy(void *obj) 
     606{ 
     607    pj_ice_strans *ice_st = (pj_ice_strans*)obj; 
     608 
     609    PJ_LOG(4,(ice_st->obj_name, "ICE stream transport %p destroyed", obj)); 
     610 
     611    /* Done */ 
     612    pj_pool_release(ice_st->pool); 
     613} 
     614 
    608615/* Destroy ICE */ 
    609616static void destroy_ice_st(pj_ice_strans *ice_st) 
    … …  
    611618    unsigned i; 
    612619 
    613     PJ_LOG(5,(ice_st->obj_name, "ICE stream transport destroying..")); 
     620    PJ_LOG(5,(ice_st->obj_name, "ICE stream transport %p destroy request..", 
     621              ice_st)); 
    614622    pj_log_push_indent(); 
     623 
     624    pj_grp_lock_acquire(ice_st->grp_lock); 
     625 
     626    if (ice_st->destroy_req) { 
     627        pj_grp_lock_release(ice_st->grp_lock); 
     628        return; 
     629    } 
     630 
     631    ice_st->destroy_req = PJ_TRUE; 
    615632 
    616633    /* Destroy ICE if we have ICE */ 
    … …  
    624641        if (ice_st->comp[i]) { 
    625642            if (ice_st->comp[i]->stun_sock) { 
    626                 pj_stun_sock_set_user_data(ice_st->comp[i]->stun_sock, NULL); 
    627643                pj_stun_sock_destroy(ice_st->comp[i]->stun_sock); 
    628644                ice_st->comp[i]->stun_sock = NULL; 
    629645            } 
    630646            if (ice_st->comp[i]->turn_sock) { 
    631                 pj_turn_sock_set_user_data(ice_st->comp[i]->turn_sock, NULL); 
    632647                pj_turn_sock_destroy(ice_st->comp[i]->turn_sock); 
    633648                ice_st->comp[i]->turn_sock = NULL; 
    … …  
    635650        } 
    636651    } 
    637     ice_st->comp_cnt = 0; 
    638  
    639     /* Destroy mutex */ 
    640     if (ice_st->init_lock) { 
    641         pj_lock_acquire(ice_st->init_lock); 
    642         pj_lock_release(ice_st->init_lock); 
    643         pj_lock_destroy(ice_st->init_lock); 
    644         ice_st->init_lock = NULL; 
    645     } 
    646  
    647     /* Destroy reference counter */ 
    648     if (ice_st->busy_cnt) { 
    649         pj_assert(pj_atomic_get(ice_st->busy_cnt)==0); 
    650         pj_atomic_destroy(ice_st->busy_cnt); 
    651         ice_st->busy_cnt = NULL; 
    652     } 
    653  
    654     PJ_LOG(4,(ice_st->obj_name, "ICE stream transport destroyed")); 
    655  
    656     /* Done */ 
    657     pj_pool_release(ice_st->pool); 
     652 
     653    pj_grp_lock_dec_ref(ice_st->grp_lock); 
     654    pj_grp_lock_release(ice_st->grp_lock); 
     655 
    658656    pj_log_pop_indent(); 
    659657} 
    … …  
    740738PJ_DEF(pj_status_t) pj_ice_strans_destroy(pj_ice_strans *ice_st) 
    741739{ 
    742     PJ_ASSERT_RETURN(ice_st, PJ_EINVAL); 
    743  
    744     sess_add_ref(ice_st); 
    745     ice_st->destroy_req = PJ_TRUE; 
    746     if (sess_dec_ref(ice_st)) { 
    747         PJ_LOG(5,(ice_st->obj_name,  
    748                   "ICE strans object is busy, will destroy later")); 
    749         return PJ_EPENDING; 
    750     } 
    751  
     740    destroy_ice_st(ice_st); 
    752741    return PJ_SUCCESS; 
    753742} 
    754743 
    755  
    756 /* 
    757  * Increment busy counter. 
    758  */ 
    759 static void sess_add_ref(pj_ice_strans *ice_st) 
    760 { 
    761     pj_atomic_inc(ice_st->busy_cnt); 
    762 } 
    763  
    764 /* 
    765  * Decrement busy counter. If the counter has reached zero and destroy 
    766  * has been requested, destroy the object and return FALSE. 
    767  */ 
    768 static pj_bool_t sess_dec_ref(pj_ice_strans *ice_st) 
    769 { 
    770     int count = pj_atomic_dec_and_get(ice_st->busy_cnt); 
    771     pj_assert(count >= 0); 
    772     if (count==0 && ice_st->destroy_req) { 
    773         destroy_ice_st(ice_st); 
    774         return PJ_FALSE; 
    775     } else { 
    776         return PJ_TRUE; 
    777     } 
    778 } 
    779744 
    780745/* 
    … …  
    841806    status = pj_ice_sess_create(&ice_st->cfg.stun_cfg, ice_st->obj_name, role, 
    842807                                ice_st->comp_cnt, &ice_cb,  
    843                                 local_ufrag, local_passwd, &ice_st->ice); 
     808                                local_ufrag, local_passwd, 
     809                                ice_st->grp_lock, 
     810                                &ice_st->ice); 
    844811    if (status != PJ_SUCCESS) 
    845812        return status; 
    … …  
    12561223    unsigned msec; 
    12571224 
    1258     sess_add_ref(ice_st); 
     1225    pj_grp_lock_add_ref(ice_st->grp_lock); 
    12591226 
    12601227    pj_gettimeofday(&t); 
    … …  
    13381305    } 
    13391306 
    1340     sess_dec_ref(ice_st); 
     1307    pj_grp_lock_dec_ref(ice_st->grp_lock); 
    13411308} 
    13421309 
    … …  
    14271394    ice_st = comp->ice_st; 
    14281395 
    1429     sess_add_ref(ice_st); 
     1396    pj_grp_lock_add_ref(ice_st->grp_lock); 
    14301397 
    14311398    if (ice_st->ice == NULL) { 
    … …  
    14521419    } 
    14531420 
    1454     return sess_dec_ref(ice_st); 
     1421    return pj_grp_lock_dec_ref(ice_st->grp_lock) ? PJ_FALSE : PJ_TRUE; 
    14551422} 
    14561423 
    … …  
    14831450    ice_st = comp->ice_st; 
    14841451 
    1485     sess_add_ref(ice_st); 
     1452    pj_grp_lock_add_ref(ice_st->grp_lock); 
    14861453 
    14871454    /* Wait until initialization completes */ 
    1488     pj_lock_acquire(ice_st->init_lock); 
     1455    pj_grp_lock_acquire(ice_st->grp_lock); 
    14891456 
    14901457    /* Find the srflx cancidate */ 
    … …  
    14961463    } 
    14971464 
    1498     pj_lock_release(ice_st->init_lock); 
     1465    pj_grp_lock_release(ice_st->grp_lock); 
    14991466 
    15001467    /* It is possible that we don't have srflx candidate even though this 
    … …  
    15031470     */ 
    15041471    if (cand == NULL) { 
    1505         return sess_dec_ref(ice_st); 
     1472        return pj_grp_lock_dec_ref(ice_st->grp_lock) ? PJ_FALSE : PJ_TRUE; 
    15061473    } 
    15071474 
    … …  
    16191586    } 
    16201587 
    1621     return sess_dec_ref(ice_st); 
     1588    return pj_grp_lock_dec_ref(ice_st->grp_lock)? PJ_FALSE : PJ_TRUE; 
    16221589} 
    16231590 
    … …  
    16381605    } 
    16391606 
    1640     sess_add_ref(comp->ice_st); 
     1607    pj_grp_lock_add_ref(comp->ice_st->grp_lock); 
    16411608 
    16421609    if (comp->ice_st->ice == NULL) { 
    … …  
    16651632    } 
    16661633 
    1667     sess_dec_ref(comp->ice_st); 
     1634    pj_grp_lock_dec_ref(comp->ice_st->grp_lock); 
    16681635} 
    16691636 
    … …  
    16871654    pj_log_push_indent(); 
    16881655 
    1689     sess_add_ref(comp->ice_st); 
     1656    pj_grp_lock_add_ref(comp->ice_st->grp_lock); 
    16901657 
    16911658    if (new_state == PJ_TURN_STATE_READY) { 
    … …  
    17011668 
    17021669        /* Wait until initialization completes */ 
    1703         pj_lock_acquire(comp->ice_st->init_lock); 
     1670        pj_grp_lock_acquire(comp->ice_st->grp_lock); 
    17041671 
    17051672        /* Find relayed candidate in the component */ 
    … …  
    17121679        pj_assert(cand != NULL); 
    17131680 
    1714         pj_lock_release(comp->ice_st->init_lock); 
     1681        pj_grp_lock_release(comp->ice_st->grp_lock); 
    17151682 
    17161683        /* Update candidate */ 
    … …  
    17451712        comp->turn_sock = NULL; 
    17461713 
    1747         /* Set session to fail if we're still initializing */ 
    1748         if (comp->ice_st->state < PJ_ICE_STRANS_STATE_READY) { 
    1749             sess_fail(comp->ice_st, PJ_ICE_STRANS_OP_INIT, 
    1750                       "TURN allocation failed", info.last_status); 
    1751         } else if (comp->turn_err_cnt > 1) { 
    1752             sess_fail(comp->ice_st, PJ_ICE_STRANS_OP_KEEP_ALIVE, 
    1753                       "TURN refresh failed", info.last_status); 
    1754         } else { 
    1755             PJ_PERROR(4,(comp->ice_st->obj_name, info.last_status, 
    1756                       "Comp %d: TURN allocation failed, retrying", 
    1757                       comp->comp_id)); 
    1758             add_update_turn(comp->ice_st, comp); 
    1759         } 
    1760     } 
    1761  
    1762     sess_dec_ref(comp->ice_st); 
     1714        /* Set session to fail on error. last_status PJ_SUCCESS means normal 
     1715         * deallocation, which should not trigger sess_fail as it may have 
     1716         * been initiated by ICE destroy 
     1717         */ 
     1718        if (info.last_status != PJ_SUCCESS) { 
     1719            if (comp->ice_st->state < PJ_ICE_STRANS_STATE_READY) { 
     1720                sess_fail(comp->ice_st, PJ_ICE_STRANS_OP_INIT, 
     1721                          "TURN allocation failed", info.last_status); 
     1722            } else if (comp->turn_err_cnt > 1) { 
     1723                sess_fail(comp->ice_st, PJ_ICE_STRANS_OP_KEEP_ALIVE, 
     1724                          "TURN refresh failed", info.last_status); 
     1725            } else { 
     1726                PJ_PERROR(4,(comp->ice_st->obj_name, info.last_status, 
     1727                          "Comp %d: TURN allocation failed, retrying", 
     1728                          comp->comp_id)); 
     1729                add_update_turn(comp->ice_st, comp); 
     1730            } 
     1731        } 
     1732    } 
     1733 
     1734    pj_grp_lock_dec_ref(comp->ice_st->grp_lock); 
    17631735 
    17641736    pj_log_pop_indent(); 
Note: See TracChangeset for help on using the changeset viewer.