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

Am 13.07.2006, 17:49 Uhr, schrieb Vincent CHAVANIS <[email protected]>:

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

no, p will never be null here because we added key with List to dictionary  
in conn_pool_put.
what would be the cause is that key got somehow corrupted... but walking  
through code I don't see how it could happens.

> 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