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_transaction.c

    r4352 r4360  
    2727 
    2828 
     29#define THIS_FILE               "stun_transaction.c" 
     30#define TIMER_INACTIVE          0 
    2931#define TIMER_ACTIVE            1 
    3032 
    … …  
    3537    pj_stun_tsx_cb       cb; 
    3638    void                *user_data; 
     39    pj_grp_lock_t       *grp_lock; 
    3740 
    3841    pj_bool_t            complete; 
    … …  
    5255 
    5356 
     57#if 1 
     58#   define TRACE_(expr)             PJ_LOG(5,expr) 
     59#else 
     60#   define TRACE_(expr) 
     61#endif 
     62 
     63 
    5464static void retransmit_timer_callback(pj_timer_heap_t *timer_heap,  
    5565                                      pj_timer_entry *timer); 
    … …  
    5767                                   pj_timer_entry *timer); 
    5868 
    59 #define stun_perror(tsx,msg,rc) pjnath_perror(tsx->obj_name, msg, rc) 
    60  
    6169/* 
    6270 * Create a STUN client transaction. 
    … …  
    6472PJ_DEF(pj_status_t) pj_stun_client_tsx_create(pj_stun_config *cfg, 
    6573                                              pj_pool_t *pool, 
     74                                              pj_grp_lock_t *grp_lock, 
    6675                                              const pj_stun_tsx_cb *cb, 
    6776                                              pj_stun_client_tsx **p_tsx) 
    … …  
    7584    tsx->rto_msec = cfg->rto_msec; 
    7685    tsx->timer_heap = cfg->timer_heap; 
     86    tsx->grp_lock = grp_lock; 
    7787    pj_memcpy(&tsx->cb, cb, sizeof(*cb)); 
    7888 
    … …  
    8393    tsx->destroy_timer.user_data = tsx; 
    8494 
    85     pj_ansi_snprintf(tsx->obj_name, sizeof(tsx->obj_name), "stuntsx%p", tsx); 
     95    pj_ansi_snprintf(tsx->obj_name, sizeof(tsx->obj_name), "utsx%p", tsx); 
    8696 
    8797    *p_tsx = tsx; 
    … …  
    101111    PJ_ASSERT_RETURN(tsx->cb.on_destroy, PJ_EINVAL); 
    102112 
     113    pj_grp_lock_acquire(tsx->grp_lock); 
     114 
    103115    /* Cancel previously registered timer */ 
    104     if (tsx->destroy_timer.id != 0) { 
    105         pj_timer_heap_cancel(tsx->timer_heap, &tsx->destroy_timer); 
    106         tsx->destroy_timer.id = 0; 
    107     } 
     116    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->destroy_timer, 
     117                                   TIMER_INACTIVE); 
    108118 
    109119    /* Stop retransmission, just in case */ 
    110     if (tsx->retransmit_timer.id != 0) { 
    111         pj_timer_heap_cancel(tsx->timer_heap, &tsx->retransmit_timer); 
    112         tsx->retransmit_timer.id = 0; 
    113     } 
    114  
    115     status = pj_timer_heap_schedule(tsx->timer_heap, 
    116                                     &tsx->destroy_timer, delay); 
    117     if (status != PJ_SUCCESS) 
     120    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->retransmit_timer, 
     121                                   TIMER_INACTIVE); 
     122 
     123    status = pj_timer_heap_schedule_w_grp_lock(tsx->timer_heap, 
     124                                               &tsx->destroy_timer, delay, 
     125                                               TIMER_ACTIVE, tsx->grp_lock); 
     126    if (status != PJ_SUCCESS) { 
     127        pj_grp_lock_release(tsx->grp_lock); 
    118128        return status; 
    119  
    120     tsx->destroy_timer.id = TIMER_ACTIVE; 
     129    } 
     130 
    121131    tsx->cb.on_complete = NULL; 
    122132 
     133    pj_grp_lock_release(tsx->grp_lock); 
     134 
     135    TRACE_((tsx->obj_name, "STUN transaction %p schedule destroy", tsx)); 
     136 
    123137    return PJ_SUCCESS; 
    124138} 
    … …  
    128142 * Destroy transaction immediately. 
    129143 */ 
    130 PJ_DEF(pj_status_t) pj_stun_client_tsx_destroy(pj_stun_client_tsx *tsx) 
     144PJ_DEF(pj_status_t) pj_stun_client_tsx_stop(pj_stun_client_tsx *tsx) 
    131145{ 
    132146    PJ_ASSERT_RETURN(tsx, PJ_EINVAL); 
    133147 
    134     if (tsx->retransmit_timer.id != 0) { 
    135         pj_timer_heap_cancel(tsx->timer_heap, &tsx->retransmit_timer); 
    136         tsx->retransmit_timer.id = 0; 
    137     } 
    138     if (tsx->destroy_timer.id != 0) { 
    139         pj_timer_heap_cancel(tsx->timer_heap, &tsx->destroy_timer); 
    140         tsx->destroy_timer.id = 0; 
    141     } 
    142  
    143     PJ_LOG(5,(tsx->obj_name, "STUN client transaction destroyed")); 
     148    /* Don't call grp_lock_acquire() because we might be called on 
     149     * group lock's destructor. 
     150     */ 
     151    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->retransmit_timer, 
     152                                   TIMER_INACTIVE); 
     153    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->destroy_timer, 
     154                                   TIMER_INACTIVE); 
     155 
     156    PJ_LOG(5,(tsx->obj_name, "STUN client transaction %p stopped, ref_cnt=%d", 
     157              tsx, pj_grp_lock_get_ref(tsx->grp_lock))); 
     158 
    144159    return PJ_SUCCESS; 
    145160} 
    … …  
    186201    pj_status_t status; 
    187202 
    188     PJ_ASSERT_RETURN(tsx->retransmit_timer.id == 0 || 
     203    PJ_ASSERT_RETURN(tsx->retransmit_timer.id == TIMER_INACTIVE || 
    189204                     !tsx->require_retransmit, PJ_EBUSY); 
    190205 
    … …  
    212227         * cancel transmission). 
    213228         */; 
    214         status = pj_timer_heap_schedule(tsx->timer_heap,  
    215                                         &tsx->retransmit_timer, 
    216                                         &tsx->retransmit_time); 
     229        status = pj_timer_heap_schedule_w_grp_lock(tsx->timer_heap, 
     230                                                   &tsx->retransmit_timer, 
     231                                                   &tsx->retransmit_time, 
     232                                                   TIMER_ACTIVE, 
     233                                                   tsx->grp_lock); 
    217234        if (status != PJ_SUCCESS) { 
    218             tsx->retransmit_timer.id = 0; 
     235            tsx->retransmit_timer.id = TIMER_INACTIVE; 
    219236            return status; 
    220237        } 
    221         tsx->retransmit_timer.id = TIMER_ACTIVE; 
    222238    } 
    223239 
    … …  
    236252        /* We've been destroyed, don't access the object. */ 
    237253    } else if (status != PJ_SUCCESS) { 
    238         if (tsx->retransmit_timer.id != 0 && mod_count) { 
    239             pj_timer_heap_cancel(tsx->timer_heap,  
    240                                  &tsx->retransmit_timer); 
    241             tsx->retransmit_timer.id = 0; 
    242         } 
    243         stun_perror(tsx, "STUN error sending message", status); 
     254        if (mod_count) { 
     255                pj_timer_heap_cancel_if_active( tsx->timer_heap, 
     256                                                &tsx->retransmit_timer, 
     257                                                TIMER_INACTIVE); 
     258        } 
     259        PJ_PERROR(4, (tsx->obj_name, status, "STUN error sending message")); 
    244260    } 
    245261 
    … …  
    261277    PJ_ASSERT_RETURN(tsx && pkt && pkt_len, PJ_EINVAL); 
    262278    PJ_ASSERT_RETURN(tsx->retransmit_timer.id == 0, PJ_EBUSY); 
     279 
     280    pj_grp_lock_acquire(tsx->grp_lock); 
    263281 
    264282    /* Encode message */ 
    … …  
    287305         * cancel transmission). 
    288306         */; 
    289         status = pj_timer_heap_schedule(tsx->timer_heap,  
    290                                         &tsx->retransmit_timer, 
    291                                         &tsx->retransmit_time); 
     307        status = pj_timer_heap_schedule_w_grp_lock(tsx->timer_heap, 
     308                                                   &tsx->retransmit_timer, 
     309                                                   &tsx->retransmit_time, 
     310                                                   TIMER_ACTIVE, 
     311                                                   tsx->grp_lock); 
    292312        if (status != PJ_SUCCESS) { 
    293             tsx->retransmit_timer.id = 0; 
     313            tsx->retransmit_timer.id = TIMER_INACTIVE; 
     314            pj_grp_lock_release(tsx->grp_lock); 
    294315            return status; 
    295316        } 
    296         tsx->retransmit_timer.id = TIMER_ACTIVE; 
    297317    } 
    298318 
    … …  
    300320    status = tsx_transmit_msg(tsx, PJ_TRUE); 
    301321    if (status != PJ_SUCCESS) { 
    302         if (tsx->retransmit_timer.id != 0) { 
    303             pj_timer_heap_cancel(tsx->timer_heap,  
    304                                  &tsx->retransmit_timer); 
    305             tsx->retransmit_timer.id = 0; 
    306         } 
     322        pj_timer_heap_cancel_if_active(tsx->timer_heap, 
     323                                       &tsx->retransmit_timer, 
     324                                       TIMER_INACTIVE); 
     325        pj_grp_lock_release(tsx->grp_lock); 
    307326        return status; 
    308327    } 
    309328 
     329    pj_grp_lock_release(tsx->grp_lock); 
    310330    return PJ_SUCCESS; 
    311331} 
    … …  
    320340 
    321341    PJ_UNUSED_ARG(timer_heap); 
     342    pj_grp_lock_acquire(tsx->grp_lock); 
    322343 
    323344    if (tsx->transmit_count >= PJ_STUN_MAX_TRANSMIT_COUNT) { 
    … …  
    332353            } 
    333354        } 
     355        pj_grp_lock_release(tsx->grp_lock); 
    334356        /* We might have been destroyed, don't try to access the object */ 
    335357        pj_log_pop_indent(); 
    … …  
    339361    tsx->retransmit_timer.id = 0; 
    340362    status = tsx_transmit_msg(tsx, PJ_TRUE); 
    341     if (status == PJNATH_ESTUNDESTROYED) { 
    342         /* We've been destroyed, don't try to access the object */ 
    343     } else if (status != PJ_SUCCESS) { 
     363    if (status != PJ_SUCCESS) { 
    344364        tsx->retransmit_timer.id = 0; 
    345365        if (!tsx->complete) { 
    … …  
    349369            } 
    350370        } 
    351         /* We might have been destroyed, don't try to access the object */ 
    352     } 
     371    } 
     372 
     373    pj_grp_lock_release(tsx->grp_lock); 
     374    /* We might have been destroyed, don't try to access the object */ 
    353375} 
    354376 
    … …  
    363385    } 
    364386 
    365     if (tsx->retransmit_timer.id != 0) { 
    366         pj_timer_heap_cancel(tsx->timer_heap, &tsx->retransmit_timer); 
    367         tsx->retransmit_timer.id = 0; 
    368     } 
     387    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->retransmit_timer, 
     388                                   TIMER_INACTIVE); 
    369389 
    370390    return tsx_transmit_msg(tsx, mod_count); 
    … …  
    380400 
    381401    tsx->destroy_timer.id = PJ_FALSE; 
     402 
    382403    tsx->cb.on_destroy(tsx); 
    383404    /* Don't access transaction after this */ 
    … …  
    409430     * We can cancel retransmit timer now. 
    410431     */ 
    411     if (tsx->retransmit_timer.id) { 
    412         pj_timer_heap_cancel(tsx->timer_heap, &tsx->retransmit_timer); 
    413         tsx->retransmit_timer.id = 0; 
    414     } 
     432    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->retransmit_timer, 
     433                                   TIMER_INACTIVE); 
    415434 
    416435    /* Find STUN error code attribute */ 
Note: See TracChangeset for help on using the changeset viewer.