Re: [PATCH] 0000326: SMSbox CRASH: PANIC: gwlib/list.c:502:lock:Assertion

Maybe i'm missing something but,

when executing  list = dict_get(conn_pool, key);

//--->
void *dict_get(Dict *dict, Octstr *key)
{
    long i;
    Item *p;
    void *value;
  
    lock(dict);
    i = key_to_index(dict, key);
    if (dict->tab[i] == NULL)
        p = NULL;
    else
        p = gwlist_search(dict->tab[i], key, item_has_key);
    if (p == NULL)
        value = NULL;
    else
        value = p->value;
    unlock(dict);
    return value;
}
//---> 

if p==NULL (the real question is here) then 'value' will be set to NULL too, then returned
so List *list will be NULLed.
In that case, gwlist_delete_equal(list, conn) will pannic.

Vincent

--
Telemaque - NICE - (FR)
Service Technique - Developpement
http://www.telemaque.fr/
[email protected]
Tel : +33 4 93 97 71 64 (fax 68)

----- Original Message ----- 
From: "Alexander Malysh" <[email protected]>
To: <[email protected]>
Sent: Thursday, July 13, 2006 5:22 PM
Subject: Re: [PATCH] 0000326: SMSbox CRASH: PANIC: gwlib/list.c:502:lock:Assertion


Hi,

just looked through code again and I really don't see how this patch  
should help and unfortunately I don't see how List in conn_pool dictionary  
may be NULL.

Thanks,
Alex


Am 13.07.2006, 01:20 Uhr, schrieb Vincent CHAVANIS <[email protected]>:

> Yep, it fixes the problem (or may fixes the problem as it does not panic  
> anymore for the moment)
>
> Someone can confirm this too ?
>
> Vincent.
>
> --
> Telemaque - NICE - (FR)
> Service Technique - Developpement
> http://www.telemaque.fr/
> [email protected]
> Tel : +33 4 93 97 71 64 (fax 68)
>
> ----- Original Message -----
> From: "Alexander Malysh" <[email protected]>
> To: <[email protected]>
> Sent: Wednesday, July 12, 2006 10:34 PM
> Subject: Re: [PATCH] 0000326: SMSbox CRASH: PANIC: gwlib/list.c:502:  
> lock:Assertion
>
>
>> Hi,
>>
>> this patch doesn't add anything and doesn't fix #326.
>> List in conn_pool dictionary in function check_pool_conn is never NULL!
>> Just look how we add Connection check to FDSet, function conn_pool_put:
>>
>>     key = conn_pool_key(host, port);
>>     mutex_lock(conn_pool_lock);
>>     list = dict_get(conn_pool, key);
>>     if (list == NULL) {
>>         list = gwlist_create();
>>         dict_put(conn_pool, key, list);
>>     }
>>     gwlist_append(list, conn);
>>     /* register connection to get server disconnect */
>>     conn_register_real(conn, client_fdset, check_pool_conn, key,
>> octstr_destroy_item);
>>
>> If this patch fix #326 then we have heavy race condition somewhere in
>> the conn.c,fdset.c,http.c.
>>
>> Could someone to 100% confirm that this patch fix #326?
>>
>> Thanks,
>> Alex
>>
>> Vincent CHAVANIS schrieb:
>>> We have here an issue :
>>>
>>>
>>> -list is not checked in gwlist_delete_equal()
>>> So if dict_get(conn_pool, key) returns NULL the process panic.
>>>
>>>
>>>
>>> diff -rau /gateway-cvs/gwlib/http.c /gateway/gwlib/http.c
>>> --- /gateway-cvs/gwlib/http.c   2006-04-01 19:45:13.000000000 +0200
>>> +++ /gateway/gwlib/http.c       2006-06-26 20:08:53.000000000 +0200
>>> @@ -848,7 +848,7 @@
>>>          List *list;
>>>          mutex_lock(conn_pool_lock);
>>>          list = dict_get(conn_pool, key);
>>> -        if (gwlist_delete_equal(list, conn) > 0) {
>>> +        if (list != NULL && gwlist_delete_equal(list, conn) > 0) {
>>>              /*
>>>               * ok, connection was still within pool. So it's
>>>               * safe to destroy this connection.
>>>
>>>
>>>
>>> --
>>> Telemaque - NICE - (FR)
>>> Service Technique - Developpement
>>> http://www.telemaque.fr/
>>> [email protected]
>>> Tel : +33 4 93 97 71 64 (fax 68)
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> diff -rau /gateway-cvs/gwlib/http.c /gateway/gwlib/http.c
>>> --- /gateway-cvs/gwlib/http.c   2006-04-01 19:45:13.000000000 +0200
>>> +++ /gateway/gwlib/http.c       2006-06-26 20:08:53.000000000 +0200
>>> @@ -848,7 +848,7 @@
>>>          List *list;
>>>          mutex_lock(conn_pool_lock);
>>>          list = dict_get(conn_pool, key);
>>> -        if (gwlist_delete_equal(list, conn) > 0) {
>>> +        if (list != NULL && gwlist_delete_equal(list, conn) > 0) {
>>>              /*
>>>               * ok, connection was still within pool. So it's
>>>               * safe to destroy this connection.
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> _______________________________________________
>>> devel mailing list
>>> [email protected]
>>> http://www.kannel.org/mailman/listinfo/devel
>>
>>
>>
>
>
>



-- 
Thanks,
Alex