Changeset 5971


Ignore:
Timestamp:
Apr 23, 2019 8:42:45 AM (6 years ago)
Author:
nanang
Message:

Fixed #2191:

  • Stricter double timer entry scheduling prevention.
  • Integrate group lock in SIP transport, e.g: for add/dec ref, for timer scheduling.


Location:
pjproject/trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • pjproject/trunk/pjlib/include/pj/timer.h

    r4567 r5971  
    253253 * @param ht        The timer heap. 
    254254 * @param entry     The entry to be registered. 
     255 * @param delay     The interval to expire. 
    255256 * @param id_val    The value to be set to the "id" field of the timer entry 
    256257 *                  once the timer is scheduled. 
    257  * @param delay     The interval to expire. 
    258258 * @param grp_lock  The group lock. 
    259259 * 

  • pjproject/trunk/pjlib/src/pj/timer.c

    r5934 r5971  
    503503 
    504504    /* Prevent same entry from being scheduled more than once */ 
    505     PJ_ASSERT_RETURN(entry->_timer_id < 1, PJ_EINVALIDOP); 
     505    //PJ_ASSERT_RETURN(entry->_timer_id < 1, PJ_EINVALIDOP); 
    506506 
    507507#if PJ_TIMER_DEBUG 
    … …  
    513513     
    514514    lock_timer_heap(ht); 
     515 
     516    /* Prevent same entry from being scheduled more than once */ 
     517    if (pj_timer_entry_running(entry)) { 
     518        unlock_timer_heap(ht); 
     519        PJ_LOG(3,(THIS_FILE, "Bug! Rescheduling outstanding entry (%p)", 
     520                  entry)); 
     521        return PJ_EINVALIDOP; 
     522    } 
     523 
    515524    status = schedule_entry(ht, entry, &expires); 
    516525    if (status == PJ_SUCCESS) { 

  • pjproject/trunk/pjsip/include/pjsip/sip_endpoint.h

    r5397 r5971  
    139139                                                const pj_time_val *max_timeout, 
    140140                                                unsigned *count); 
     141 
    141142/** 
    142143 * Schedule timer to endpoint's timer heap. Application must poll the endpoint 
    … …  
    165166                                                 pj_timer_entry *entry, 
    166167                                                 const pj_time_val *delay ); 
     168#endif 
     169 
     170/** 
     171 * Schedule timer to endpoint's timer heap with group lock. Application must 
     172 * poll the endpoint periodically (by calling #pjsip_endpt_handle_events) to 
     173 * ensure that the timer events are handled in timely manner. When the 
     174 * timeout for the timer has elapsed, the callback specified in the entry 
     175 * argument will be called. This function, like all other endpoint functions, 
     176 * is thread safe. 
     177 * 
     178 * @param endpt     The endpoint. 
     179 * @param entry     The timer entry. 
     180 * @param delay     The relative delay of the timer. 
     181 * @param id_val    The value to be set to the "id" field of the timer entry 
     182 *                  once the timer is scheduled. 
     183 * @param grp_lock  The group lock. 
     184 * @return          PJ_OK (zero) if successfull. 
     185 */ 
     186#if PJ_TIMER_DEBUG 
     187#define pjsip_endpt_schedule_timer_w_grp_lock(ept,ent,d,id,gl) \ 
     188                pjsip_endpt_schedule_timer_w_grp_lock_dbg(ept,ent,d,id,gl,\ 
     189                                                          __FILE__, __LINE__) 
     190 
     191PJ_DECL(pj_status_t) pjsip_endpt_schedule_timer_w_grp_lock_dbg( 
     192                                                    pjsip_endpoint *endpt, 
     193                                                    pj_timer_entry *entry, 
     194                                                    const pj_time_val *delay, 
     195                                                    int id_val, 
     196                                                    pj_grp_lock_t *grp_lock, 
     197                                                    const char *src_file, 
     198                                                    int src_line); 
     199#else 
     200PJ_DECL(pj_status_t) pjsip_endpt_schedule_timer_w_grp_lock( 
     201                                                 pjsip_endpoint *endpt, 
     202                                                 pj_timer_entry *entry, 
     203                                                 const pj_time_val *delay, 
     204                                                 int id_val, 
     205                                                 pj_grp_lock_t *grp_lock ); 
    167206#endif 
    168207 

  • pjproject/trunk/pjsip/include/pjsip/sip_transport.h

    r5884 r5971  
    811811    pj_atomic_t            *ref_cnt;        /**< Reference counter.         */ 
    812812    pj_lock_t              *lock;           /**< Lock object.               */ 
     813    pj_grp_lock_t          *grp_lock;       /**< Group lock for sync with 
     814                                                 ioqueue and timer.         */ 
    813815    pj_bool_t               tracing;        /**< Tracing enabled?           */ 
    814816    pj_bool_t               is_shutdown;    /**< Being shutdown?            */ 

  • pjproject/trunk/pjsip/src/pjsip/sip_endpoint.c

    r5668 r5971  
    804804 
    805805/* 
     806 * Schedule timer with group lock. 
     807 */ 
     808#if PJ_TIMER_DEBUG 
     809PJ_DEF(pj_status_t) pjsip_endpt_schedule_timer_w_grp_lock_dbg( 
     810                                                    pjsip_endpoint *endpt, 
     811                                                    pj_timer_entry *entry, 
     812                                                    const pj_time_val *delay, 
     813                                                    int id_val, 
     814                                                    pj_grp_lock_t *grp_lock, 
     815                                                    const char *src_file, 
     816                                                    int src_line) 
     817{ 
     818    PJ_LOG(6, (THIS_FILE, "pjsip_endpt_schedule_timer_w_grp_lock" 
     819                          "(entry=%p, delay=%u.%u, grp_lock=%p)", 
     820                          entry, delay->sec, delay->msec, grp_lock)); 
     821    return pj_timer_heap_schedule_w_grp_lock_dbg(endpt->timer_heap, entry, 
     822                                                 delay, id_val, grp_lock, 
     823                                                 src_file, src_line); 
     824} 
     825#else 
     826PJ_DEF(pj_status_t) pjsip_endpt_schedule_timer_w_grp_lock( 
     827                                                 pjsip_endpoint *endpt, 
     828                                                 pj_timer_entry *entry, 
     829                                                 const pj_time_val *delay, 
     830                                                 int id_val, 
     831                                                 pj_grp_lock_t *grp_lock ) 
     832{ 
     833    PJ_LOG(6, (THIS_FILE, "pjsip_endpt_schedule_timer_w_grp_lock" 
     834                          "(entry=%p, delay=%u.%u, grp_lock=%p)", 
     835                          entry, delay->sec, delay->msec, grp_lock)); 
     836    return pj_timer_heap_schedule_w_grp_lock( endpt->timer_heap, entry, 
     837                                              delay, id_val, grp_lock ); 
     838} 
     839#endif 
     840 
     841/* 
    806842 * Cancel the previously registered timer. 
    807843 */ 

  • pjproject/trunk/pjsip/src/pjsip/sip_transport.c

    r5909 r5971  
    10131013    PJ_UNUSED_ARG(timer_heap); 
    10141014 
     1015    if (entry->id == PJ_FALSE) 
     1016        return; 
     1017 
    10151018    entry->id = PJ_FALSE; 
    10161019    pjsip_transport_destroy(tp); 
    … …  
    10491052 
    10501053    PJ_ASSERT_RETURN(tp != NULL, PJ_EINVAL); 
     1054 
     1055    /* Add ref transport group lock, if any */ 
     1056    if (tp->grp_lock) 
     1057        pj_grp_lock_add_ref(tp->grp_lock); 
    10511058 
    10521059    /* Cache some vars for checking transport validity later */ 
    … …  
    10641071        { 
    10651072            if (tp->idle_timer.id != PJ_FALSE) { 
     1073                tp->idle_timer.id = PJ_FALSE; 
    10661074                pjsip_endpt_cancel_timer(tp->tpmgr->endpt, &tp->idle_timer); 
    1067                 tp->idle_timer.id = PJ_FALSE; 
    10681075            } 
    10691076        } 
    … …  
    11151122            } 
    11161123 
    1117             pj_assert(tp->idle_timer.id == 0); 
    1118             tp->idle_timer.id = PJ_TRUE; 
    1119             pjsip_endpt_schedule_timer(tp->tpmgr->endpt, &tp->idle_timer,  
    1120                                        &delay); 
     1124            /* Avoid double timer entry scheduling */ 
     1125            if (pj_timer_entry_running(&tp->idle_timer)) 
     1126                pjsip_endpt_cancel_timer(tp->tpmgr->endpt, &tp->idle_timer); 
     1127 
     1128            pjsip_endpt_schedule_timer_w_grp_lock(tp->tpmgr->endpt, 
     1129                                                  &tp->idle_timer, 
     1130                                                  &delay, 
     1131                                                  PJ_TRUE, 
     1132                                                  tp->grp_lock); 
    11211133        } 
    11221134        pj_lock_release(tpmgr->lock); 
    11231135    } 
     1136 
     1137    /* Dec ref transport group lock, if any */ 
     1138    if (tp->grp_lock) 
     1139        pj_grp_lock_dec_ref(tp->grp_lock); 
    11241140 
    11251141    return PJ_SUCCESS; 
    … …  
    11691185    pj_hash_set(tp->pool, mgr->table, &tp->key, key_len, hval, tp); 
    11701186 
     1187    /* Add ref transport group lock, if any */ 
     1188    if (tp->grp_lock) 
     1189        pj_grp_lock_add_ref(tp->grp_lock); 
     1190 
    11711191    pj_lock_release(mgr->lock); 
    11721192 
    … …  
    12001220    //pj_assert(tp->idle_timer.id == PJ_FALSE); 
    12011221    if (tp->idle_timer.id != PJ_FALSE) { 
     1222        tp->idle_timer.id = PJ_FALSE; 
    12021223        pjsip_endpt_cancel_timer(mgr->endpt, &tp->idle_timer); 
    1203         tp->idle_timer.id = PJ_FALSE; 
    12041224    } 
    12051225 
    … …  
    12261246    pj_lock_release(mgr->lock); 
    12271247    pj_lock_release(tp->lock); 
     1248 
     1249    /* Dec ref transport group lock, if any */ 
     1250    if (tp->grp_lock) 
     1251        pj_grp_lock_dec_ref(tp->grp_lock); 
    12281252 
    12291253    /* Destroy. */ 

  • pjproject/trunk/pjsip/src/pjsip/sip_transport_tcp.c

    r5870 r5971  
    693693    pj_grp_lock_add_handler(tcp->grp_lock, pool, tcp, &tcp_on_destroy); 
    694694 
     695    tcp->base.grp_lock = tcp->grp_lock; 
     696 
    695697    /* Create active socket */ 
    696698    pj_activesock_cfg_default(&asock_cfg); 
    … …  
    747749 
    748750on_error: 
    749     tcp_destroy(&tcp->base, status); 
     751    if (tcp->grp_lock && pj_grp_lock_get_ref(tcp->grp_lock)) 
     752        tcp_destroy(&tcp->base, status); 
     753    else 
     754        tcp_on_destroy(tcp); 
     755 
    750756    return status; 
    751757} 
    … …  
    868874        pj_grp_lock_dec_ref(grp_lock); 
    869875        /* Transport may have been deleted at this point */ 
    870     } else { 
    871         tcp_on_destroy(tcp); 
    872876    } 
    873877 

  • pjproject/trunk/pjsip/src/pjsip/sip_transport_tls.c

    r5936 r5971  
    164164                              const pj_str_t *remote_name, 
    165165                              struct tls_transport **p_tls); 
     166 
     167 
     168/* Clean up TLS resources */ 
     169static void tls_on_destroy(void *arg); 
    166170 
    167171 
    … …  
    894898 
    895899on_error: 
    896     tls_destroy(&tls->base, status); 
     900    if (tls->grp_lock && pj_grp_lock_get_ref(tls->grp_lock)) 
     901        tls_destroy(&tls->base, status); 
     902    else 
     903        tls_on_destroy(tls); 
     904 
    897905    return status; 
    898906} 
    … …  
    10491057        pj_grp_lock_dec_ref(grp_lock); 
    10501058        /* Transport may have been deleted at this point */ 
    1051     } else { 
    1052         tls_on_destroy(tls); 
    10531059    } 
    10541060 
    … …  
    12361242 
    12371243    /* Set up the group lock */ 
    1238     tls->grp_lock = glock; 
     1244    tls->grp_lock = tls->base.grp_lock = glock; 
    12391245    pj_grp_lock_add_ref(tls->grp_lock); 
    12401246    pj_grp_lock_add_handler(tls->grp_lock, pool, tls, &tls_on_destroy); 

  • pjproject/trunk/pjsip/src/pjsip/sip_transport_udp.c

    r5911 r5971  
    692692        pj_grp_lock_add_handler(tp->grp_lock, tp->base.pool, tp, 
    693693                                &udp_on_destroy); 
     694 
     695        tp->base.grp_lock = tp->grp_lock; 
    694696    } 
    695697     
Note: See TracChangeset for help on using the changeset viewer.