As I see it this is the most important change:
> (void)pthread_mutex_lock(&arg_mut);
>
> res->next = NULL;
>
> if (last == NULL) {
res->next isn't set to null in the 2.6c code.
Joe
> -----Original Message-----
> From: Robert Segall [mailto:[email protected]]
> Sent: Monday, March 07, 2011 3:38 AM
> To: [email protected]
> Subject: Re: [Pound Mailing List] Queue implementation errors?
>
> On Wed, 2011-02-16 at 16:40 +0000, Steven van der Vegt wrote:
> > Hi Folks,
> >
> > I wanted to test the 2.6c version on a testsystem of ours but I ran
> into trouble. Every time 1 request was handled I got this error:
> >
> > MONITOR: worker exited on signal 11, restarting...
> >
> > After some research I found out it was the memcopy in do_http() which
> causes the error:
> >
> > from_host = ((thr_arg *)arg)->from_host;
> > memcpy(&from_host_addr, from_host.ai_addr, from_host.ai_addrlen);
> > from_host.ai_addr = (struct sockaddr *)&from_host_addr;
> >
> > The value from_host.ai_addrlen was equal to 5394258 and after some
> debugging it seems the whole struct contained bogus.
> >
> > Ok, let's find out why?
> >
> > The do_http() function is called from thr_http() which has some
> strange while loop in it. It fetches work from a queue with the call
> get_thr_arg() but this call is allowed to return a NULL value
> sometimes. If this happens than ask again...
> > Well, this wasn't a NULL value so there must be something else going
> on. Maybe the queue?
> >
> > The code contained some errors (as far as my concurrent and queue
> knowledge go's). I reimplemented them to the following code. It solved
> my errors and I think the strange while-loop in the thr_http function
> is not needed anymore.
> >
> > Can you please comment on my changes?
> >
> > int
> > put_thr_arg(thr_arg *arg)
> > {
> > thr_arg *res;
> >
> > if((res = malloc(sizeof(thr_arg))) == NULL) {
> > logmsg(LOG_WARNING, "thr_arg malloc");
> > return -1;
> > }
> > memcpy(res, arg, sizeof(thr_arg));
> > (void)pthread_mutex_lock(&arg_mut);
> >
> > res->next = NULL;
> >
> > if (last == NULL) {
> > first = last = res;
> > } else {
> > last->next = res;
> > last = res;
> > }
> >
> > pthread_cond_broadcast(&arg_cond);
> > (void)pthread_mutex_unlock(&arg_mut);
> > return 0;
> > }
> >
> > /*
> > * get a request from the queue
> > */
> > thr_arg *
> > get_thr_arg(void)
> > {
> > thr_arg *res = NULL;
> >
> > (void)pthread_mutex_lock(&arg_mut);
> > while (first == NULL)
> > (void)pthread_cond_wait(&arg_cond, &arg_mut);
> >
> > res = first;
> > if((first = res->next) == NULL)
> > last = NULL;
> > else
> > pthread_cond_signal(&arg_cond);
> >
> > (void)pthread_mutex_unlock(&arg_mut);
> >
> > return res;
> > }
> >
> > Thanks!
> >
> > Kind regards,
> >
> > Steven van der Vegt
> >
> > --
> > To unsubscribe send an email with subject unsubscribe to
> [email protected].
> > Please contact [email protected] for questions.
>
> >From what I see, you have made two changes:
>
> 1. you have moved the NULL check from the caller in http.c to the
> getter
> in pound.c (a NULL check is required in any case, as the condition
> wake-up may occur even when there is no data to be consumed)
>
> 2. you have replaced the cond_signal with a cond_broadcast in the
> enqueing function
>
> The first change should make no difference (it is just moving the
> actual
> check to another place). The second will cause more threads to wake-up,
> quite often more than are required. I must admit am not at all clear
> how
> should this help.
> --
> ?Robert Segall
> Apsis GmbH
> Postfach, Uetikon am See, CH-8707
> Tel: +41-32-512 30 19
>
>
> --
> To unsubscribe send an email with subject unsubscribe to
> [email protected].
> Please contact [email protected] for questions.
--
To unsubscribe send an email with subject unsubscribe to [email protected].
Please contact [email protected] for questions.
