Ignore:
Timestamp:
Mar 25, 2006 10:06:00 AM (19 years ago)
Author:
bennylp
Message:

Fixed bug in ioqueue: crashed when key is unregistered while another thread is running a callback

File:
1 edited

Legend:

Unmodified
Added
Removed

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

    r126 r363  
    2828 */ 
    2929 
     30static long ioqueue_tls_id = -1; 
     31 
     32typedef struct key_lock_data { 
     33    struct key_lock_data *prev; 
     34    pj_ioqueue_key_t     *key; 
     35    int                   is_alive; 
     36} key_lock_data; 
     37 
     38 
    3039static void ioqueue_init( pj_ioqueue_t *ioqueue ) 
    3140{ 
    3241    ioqueue->lock = NULL; 
    3342    ioqueue->auto_delete_lock = 0; 
     43 
     44    if (ioqueue_tls_id == -1) { 
     45        pj_status_t status; 
     46        status = pj_thread_local_alloc(&ioqueue_tls_id); 
     47        pj_thread_local_set(ioqueue_tls_id, NULL); 
     48    } 
    3449} 
    3550 
    … …  
    93108        key->fd_type = PJ_SOCK_STREAM; 
    94109 
     110    key->inside_callback = 0; 
     111    key->destroy_requested = 0; 
     112 
    95113    /* Create mutex for the key. */ 
    96     rc = pj_mutex_create_simple(pool, NULL, &key->mutex); 
     114    rc = pj_mutex_create_recursive(pool, NULL, &key->mutex); 
    97115     
    98116    return rc; 
    99117} 
    100118 
     119/* Lock the key and also keep the lock data in thread local storage. 
     120 * The lock data is used to detect if the key is deleted by application 
     121 * when we call its callback. 
     122 */ 
     123static void lock_key(pj_ioqueue_key_t *key, key_lock_data *lck) 
     124{ 
     125    struct key_lock_data *prev_data; 
     126 
     127    pj_mutex_lock(key->mutex); 
     128    prev_data = (struct key_lock_data *)  
     129                    pj_thread_local_get(ioqueue_tls_id); 
     130    lck->prev = prev_data; 
     131    lck->key = key; 
     132    lck->is_alive = 1; 
     133    pj_thread_local_set(ioqueue_tls_id, lck); 
     134} 
     135 
     136/* Unlock the key only if it is still valid. */ 
     137static void unlock_key(pj_ioqueue_key_t *key, key_lock_data *lck) 
     138{ 
     139    pj_assert( (void*)pj_thread_local_get(ioqueue_tls_id) == lck); 
     140    pj_assert( lck->key == key ); 
     141    pj_thread_local_set(ioqueue_tls_id, lck->prev); 
     142    if (lck->is_alive) 
     143        pj_mutex_unlock(key->mutex); 
     144} 
     145 
     146/* Destroy key */ 
    101147static void ioqueue_destroy_key( pj_ioqueue_key_t *key ) 
    102148{ 
     149    key_lock_data *lck; 
     150 
     151    /* Make sure that no other threads are doing something with 
     152     * the key. 
     153     */ 
     154    pj_mutex_lock(key->mutex); 
     155     
     156    /* Check if this function is called within a callback context. 
     157     * If so, then we need to inform the callback that the key has 
     158     * been destroyed so that it doesn't attempt to unlock the 
     159     * key's mutex. 
     160     */ 
     161    lck = pj_thread_local_get(ioqueue_tls_id); 
     162    while (lck) { 
     163        if (lck->key == key) { 
     164            lck->is_alive = 0; 
     165        } 
     166        lck = lck->prev; 
     167    } 
     168 
    103169    pj_mutex_destroy(key->mutex); 
    104170} 
    … …  
    164230void ioqueue_dispatch_write_event(pj_ioqueue_t *ioqueue, pj_ioqueue_key_t *h) 
    165231{ 
     232    key_lock_data lck_data; 
     233 
    166234    /* Lock the key. */ 
    167     pj_mutex_lock(h->mutex); 
     235    lock_key(h, &lck_data); 
    168236 
    169237#if defined(PJ_HAS_TCP) && PJ_HAS_TCP!=0 
    … …  
    179247 
    180248        /* Unlock; from this point we don't need to hold key's mutex. */ 
    181         pj_mutex_unlock(h->mutex); 
     249        //pj_mutex_unlock(h->mutex); 
    182250 
    183251#if (defined(PJ_HAS_SO_ERROR) && PJ_HAS_SO_ERROR!=0) 
    … …  
    252320                ioqueue_remove_from_set(ioqueue, h->fd, WRITEABLE_EVENT); 
    253321 
    254             pj_mutex_unlock(h->mutex); 
     322            //pj_mutex_unlock(h->mutex); 
    255323        } 
    256324 
    … …  
    299367 
    300368                /* No need to hold mutex anymore */ 
    301                 pj_mutex_unlock(h->mutex); 
     369                //pj_mutex_unlock(h->mutex); 
    302370            } 
    303371 
    … …  
    310378 
    311379        } else { 
    312             pj_mutex_unlock(h->mutex); 
     380            //pj_mutex_unlock(h->mutex); 
    313381        } 
    314382 
    … …  
    320388         * able to process the event. 
    321389         */ 
    322         pj_mutex_unlock(h->mutex); 
    323     } 
     390        //pj_mutex_unlock(h->mutex); 
     391    } 
     392 
     393    /* Finally unlock key */ 
     394    unlock_key(h, &lck_data); 
    324395} 
    325396 
    326397void ioqueue_dispatch_read_event( pj_ioqueue_t *ioqueue, pj_ioqueue_key_t *h ) 
    327398{ 
     399    key_lock_data lck_data; 
    328400    pj_status_t rc; 
    329401 
    330402    /* Lock the key. */ 
    331     pj_mutex_lock(h->mutex); 
     403    lock_key(h, &lck_data); 
    332404 
    333405#   if PJ_HAS_TCP 
    … …  
    346418 
    347419        /* Unlock; from this point we don't need to hold key's mutex. */ 
    348         pj_mutex_unlock(h->mutex); 
     420        //pj_mutex_unlock(h->mutex); 
    349421 
    350422        rc=pj_sock_accept(h->fd, accept_op->accept_fd,  
    … …  
    379451 
    380452        /* Unlock; from this point we don't need to hold key's mutex. */ 
    381         pj_mutex_unlock(h->mutex); 
     453        //Crash as of revision 353 (since we added pjmedia socket to 
     454        //main ioqueue). 
     455        //pj_mutex_unlock(h->mutex); 
    382456 
    383457        bytes_read = read_op->size; 
    … …  
    456530         * able to process the event. 
    457531         */ 
    458         pj_mutex_unlock(h->mutex); 
    459     } 
     532        //pj_mutex_unlock(h->mutex); 
     533    } 
     534 
     535    /* Unlock handle. */ 
     536    unlock_key(h, &lck_data); 
    460537} 
    461538 
    … …  
    464541                                       pj_ioqueue_key_t *h ) 
    465542{ 
    466     pj_mutex_lock(h->mutex); 
     543    key_lock_data lck_data; 
     544 
     545    lock_key(h, &lck_data); 
    467546 
    468547    if (!h->connecting) { 
    … …  
    471550         * it has been processed by other thread. 
    472551         */ 
    473         pj_mutex_unlock(h->mutex); 
     552        //pj_mutex_unlock(h->mutex); 
     553        unlock_key(h, &lck_data); 
    474554        return; 
    475555    } 
    … …  
    478558    h->connecting = 0; 
    479559 
    480     pj_mutex_unlock(h->mutex); 
     560    //pj_mutex_unlock(h->mutex); 
    481561 
    482562    ioqueue_remove_from_set(ioqueue, h->fd, WRITEABLE_EVENT); 
    … …  
    486566    if (h->cb.on_connect_complete) 
    487567        (*h->cb.on_connect_complete)(h, -1); 
     568 
     569    unlock_key(h, &lck_data); 
    488570} 
    489571 
    … …  
    905987{ 
    906988    struct generic_operation *op_rec; 
     989    key_lock_data lck_data; 
    907990 
    908991    /* 
    … …  
    910993     * really make sure that it's still there; then call the callback. 
    911994     */ 
    912     pj_mutex_lock(key->mutex); 
     995    lock_key(key, &lck_data); 
    913996 
    914997    /* Find the operation in the pending read list. */ 
    … …  
    9181001            pj_list_erase(op_rec); 
    9191002            op_rec->op = 0; 
    920             pj_mutex_unlock(key->mutex); 
     1003            //pj_mutex_unlock(key->mutex); 
    9211004 
    9221005            (*key->cb.on_read_complete)(key, op_key, bytes_status); 
     1006 
     1007            unlock_key(key, &lck_data); 
    9231008            return PJ_SUCCESS; 
    9241009        } 
    … …  
    9321017            pj_list_erase(op_rec); 
    9331018            op_rec->op = 0; 
    934             pj_mutex_unlock(key->mutex); 
     1019            //pj_mutex_unlock(key->mutex); 
    9351020 
    9361021            (*key->cb.on_write_complete)(key, op_key, bytes_status); 
     1022 
     1023            unlock_key(key, &lck_data); 
    9371024            return PJ_SUCCESS; 
    9381025        } 
    … …  
    9461033            pj_list_erase(op_rec); 
    9471034            op_rec->op = 0; 
    948             pj_mutex_unlock(key->mutex); 
     1035            //pj_mutex_unlock(key->mutex); 
    9491036 
    9501037            (*key->cb.on_accept_complete)(key, op_key,  
    9511038                                          PJ_INVALID_SOCKET, 
    9521039                                          bytes_status); 
     1040 
     1041            unlock_key(key, &lck_data); 
    9531042            return PJ_SUCCESS; 
    9541043        } 
    … …  
    9561045    } 
    9571046 
    958     pj_mutex_unlock(key->mutex); 
     1047    unlock_key(key, &lck_data); 
    9591048     
    9601049    return PJ_EINVALIDOP; 
Note: See TracChangeset for help on using the changeset viewer.