Assertion failed in request_list.c

classic Classic list List threaded Threaded
6 messages Options
| Threaded
Open this post in threaded view
|

Assertion failed in request_list.c

Alexander Voropay
Hi!

 Under relative heavy load (~5auth/sec) I have such error every 10..15min :

Thu Aug 11 17:10:51 2005 : Auth: Login OK: [user] (from client 748/443 port 433 cli 3062541)
Thu Aug 11 17:10:54 2005 : Error: Assertion failed in request_list.c, line 577
Thu Aug 11 17:11:04 2005 : Info: Ready to process requests.

 `radwatch` kiks `radiusd` restart but I have a pause about ~5 sec while new
radiusd starts.

 There is no this problem under lite load.

 The version is about ~ 2005.06... CVS snapshot.

 Is this problem known and corrected in the latest radiusd CVS ?

 How to debug this problem ?

==== request_list.c =======
569:/*
570: *      Add a request to the request list.
571:*/
572:void rl_add(request_list_t *rl, REQUEST *request)
573:{
574:        int id = request->packet->id;
575:        REQNODE *node;
576:
577:        rad_assert(request->container == NULL);
578:
579:        request->container = rad_malloc(sizeof(REQNODE));
580:        node = (REQNODE *) request->container;
581:        node->req = request;
582:
583:        node->prev = NULL;
584:        node->next = NULL;
...
===================

--
-=AV=-
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Assertion failed in request_list.c

Ruttmann
Hi,

some time ago I expirienced the same problem. I think the problem is located
in main/threads.c line 256, where the threadlist is reallocated:

memset(new_queue + sizeof(*new_queue) * thread_pool.queue_size,
  0, sizeof(*new_queue) * thread_pool.queue_size);

the pointer new_queue is typed, the add operator imlpizit multiplies with the
size of *new_queue and the code doubles the multiplication.
=> memset zeros an area far behind the queue
=> the queue itself contains bad pointers

Solution remove the first 'sizeof(*new_queue)'

Regard: This solves only half of the problem.
The queue has moving start and end. After reallocation the queue_size is
adjusted, but queue_tail may still point in the middle of the list, so you'll
override entries.

Solution: after memset:
if queue_tail < queue_head (very likely) then move
memory range from [0, queue_tail] up to memory range [oldqueuesize,
oldqueuesize + queue_tail]
don't forget to adjust queue_tail to the new end.
Of course, you can also move from [start, oldqueuesize] up to the end of the
new list and then adjust start

Greetings

Matthias Ruttmann

P.S. I'm not used to update in CVS and forgot to report the problem before, so
please someone else implement and test this.


Am Donnerstag, 11. August 2005 15:29 schrieb Alexander Voropay:

> Hi!
>
>  Under relative heavy load (~5auth/sec) I have such error every 10..15min :
>
> Thu Aug 11 17:10:51 2005 : Auth: Login OK: [user] (from client 748/443 port
> 433 cli 3062541) Thu Aug 11 17:10:54 2005 : Error: Assertion failed in
> request_list.c, line 577 Thu Aug 11 17:11:04 2005 : Info: Ready to process
> requests.
>
>  `radwatch` kiks `radiusd` restart but I have a pause about ~5 sec while
> new radiusd starts.
>
>  There is no this problem under lite load.
>
>  The version is about ~ 2005.06... CVS snapshot.
>
>  Is this problem known and corrected in the latest radiusd CVS ?
>
>  How to debug this problem ?
>
> ==== request_list.c =======
> 569:/*
> 570: *      Add a request to the request list.
> 571:*/
> 572:void rl_add(request_list_t *rl, REQUEST *request)
> 573:{
> 574:        int id = request->packet->id;
> 575:        REQNODE *node;
> 576:
> 577:        rad_assert(request->container == NULL);
> 578:
> 579:        request->container = rad_malloc(sizeof(REQNODE));
> 580:        node = (REQNODE *) request->container;
> 581:        node->req = request;
> 582:
> 583:        node->prev = NULL;
> 584:        node->next = NULL;
> ...
> ===================
>
> --
> -=AV=-
> -
> List info/subscribe/unsubscribe? See
> http://www.freeradius.org/list/devel.html

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Assertion failed in request_list.c

Alan DeKok
Matthias Ruttmann <[hidden email]> wrote:
> some time ago I expirienced the same problem. I think the problem is located
> in main/threads.c line 256, where the threadlist is reallocated:

  I think you're right.

> the pointer new_queue is typed, the add operator imlpizit multiplies with the
> size of *new_queue and the code doubles the multiplication.
> => memset zeros an area far behind the queue

  I don't agree.  That code is OK.

> Regard: This solves only half of the problem.
> The queue has moving start and end. After reallocation the
> queue_size is adjusted, but queue_tail may still point in the middle
> of the list, so you'll override entries.

  That's the real cause of the problem.

  I'll add a patch in.  I'd like to release 1.0.5 this week, if
possible.

  Alan DeKok.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Assertion failed in request_list.c

Ruttmann
> > the pointer new_queue is typed, the add operator imlpizit multiplies with
> > the size of *new_queue and the code doubles the multiplication.
> > => memset zeros an area far behind the queue
>
>   I don't agree.  That code is OK.
>

Just to avoid misunderstanding: Of course the problem is solved as soon as the
queue start/end is adjusted properly, because the uninitialized pointers
aren't used any more.

But also the memset statement itself is errornous. Currently the code uses
case 4, but it should use case 3.

#include <stdio.h>
int main(int argc, char ** argv)
{
        struct struct Stest {
                int x1;
                int x2;
        } t[200];
        printf("case 1: Stest: %p\n", t);
        printf("case 2: &Stest[100]: %p\n", &t[100]);
        printf("case 3: Stest + 100: %p\n", t + 100);
        printf("case 4: Stest + 100 * sizeof(Stest): %p\n", t + 100 * sizeof(*t));
        printf("case 5: Stest + 800: %p\n", t + 800);
        return 0;
}

Matthias Ruttmann

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Assertion failed in request_list.c

Alan DeKok
Matthias Ruttmann <[hidden email]> wrote:
> Just to avoid misunderstanding: Of course the problem is solved as soon as the
> queue start/end is adjusted properly, because the uninitialized pointers
> aren't used any more.

  That's right.

> But also the memset statement itself is errornous. Currently the code uses
> case 4, but it should use case 3.

  Ah, I see what you mean.  I'll commit a fix for that.

  Alan DeKok.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Assertion failed in request_list.c

Alan DeKok
In reply to this post by Ruttmann
Matthias Ruttmann <[hidden email]> wrote:
> Solution: after memset:
> if queue_tail < queue_head (very likely) then move
> memory range from [0, queue_tail] up to memory range [oldqueuesize,
> oldqueuesize + queue_tail]
> don't forget to adjust queue_tail to the new end.

  Can you test the "release_1_0" branch from CVS?  I've committed a
fix which should solve this.  If it works, we can go ahead with 1.0.5.

  Alan DeKok.

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html