Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Mailbox-Line: From jlewalle@cs.ucr.edu  Fri Feb 21 10:04:02 2003
Message-ID: <3E566A1F.70701@cs.ucr.edu>
Date: Fri, 21 Feb 2003 10:04:15 -0800
From: Jacob Lewallen <jlewalle@cs.ucr.edu>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20030203
MIME-Version: 1.0
To: Jacob Lewallen <jlewalle@cs.ucr.edu>
Cc: dev@apr.apache.org
Subject: Re: [PATCH] Problem with the queues
References: <A1C30C1C0FB8D5118D810004754C037575ADF0@hsl-lj3x.hermes.si>
 <3E56688B.5060407@cs.ucr.edu>
In-Reply-To: <3E56688B.5060407@cs.ucr.edu>
Content-Type: multipart/mixed;
 boundary="------------040901020700020509000906"

This is a multi-part message in MIME format.
--------------040901020700020509000906
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Jacob Lewallen wrote:

> Some more problems with apr_queue. . .
>
> Erwin De Wolff wrote:
>
>> My application uses queues and I notice that I had some difficulties 
>> when
>> reaching the end and start of the queue. I'm using the latest (21 
>> febr 2003)
>> snapshot of apr and apr-util. Platforms on which it occurs, Windows and
>> Linux. To make the problem more accessible I use most of the same 
>> code as
>> the testqueue.c provided in the distribution. Only to simplify I use one
>> producer and one consumer only and a queue of size 1. The producer 
>> pushes
>> values 0,1,2,3,... and the consumer pops them one by one. The following
>> output is obtained:
>>
>>     0: value 0 succesfully pushed.
>>     1: value 1 succesfully pushed.
>>             0: value 1 succesfully popped.
>>     2: value 2 succesfully pushed.
>>            1: value 2 succesfully popped.
>>     3: value 3 succesfully pushed.
>>            2: value 3 succesfully popped.
>>             3: value 3 succesfully popped.
>>
>> You can notice a few things:
>> 1. value 0 is never popped
>> 2. there are two values pushed on a queue of size 1!
>> 3. value 3 is popped twice on a queue of size 1.
>>
>> I don't think I made a mistake in the code, but just for those who might
>> doubt that, I attached the used code.
>>  
>>
> [code removed]
>
> Here's what's going on here. With a queue size of 1, there's only one 
> position in the queue for placing data. In apr_queue_pop, we return 
> the address of the popped item "in the queue's own array" (hence us 
> having to dereference twice to get our integer pointer above). So 
> here's how things breakdown:
>
> P: we push 0, its placed in data[0]
> P: we try to push 1, its blocked, because we're full
> C: we pop, and are returned the address of data[0] (in our case, a 
> pointer to where the pointer to 0 is zero)
> P: unblock, push 1 into the queue (overwriting data[0] with the new 
> pointer)
> C: we dereference the pointer to data[0] that was returned, which has 
> since been replaced with a pointer to 1.
>
> And this goes on. I think that this is responsible for all of the 
> behavior we see above. I'm almost positive that I've seen someone 
> complain about how the apr_queue code returns the address of the item, 
> rather than the value itself. This is what the attached patch does, if 
> anybody has any reason for keeping the existing behavior, I'd love to 
> hear them. This, of course, breaks any code that uses apr_queue_pop, 
> so it's a big deal.
>
> jacob lewallen
> jlewalle@cs.ucr.edu
>


--------------040901020700020509000906
Content-Type: text/plain;
 name="apr_queue_overwrite_fix.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="apr_queue_overwrite_fix.diff"

? apr_queue_overwrite_fix
Index: apr_queue.c
===================================================================
RCS file: /home/cvspublic/apr-util/misc/apr_queue.c,v
retrieving revision 1.11
diff -u -u -r1.11 apr_queue.c
--- apr_queue.c	14 Feb 2003 04:39:07 -0000	1.11
+++ apr_queue.c	21 Feb 2003 17:50:34 -0000
@@ -333,7 +333,7 @@
         }
     } 
 
-    *data = &queue->data[queue->out];
+    *data = queue->data[queue->out];
     queue->nelts--;
 
     queue->out = (queue->out + 1) % queue->bounds;
@@ -375,7 +375,7 @@
         return APR_EAGAIN;
     } 
 
-    *data = &queue->data[queue->out];
+    *data = queue->data[queue->out];
     queue->nelts--;
 
     queue->out = (queue->out + 1) % queue->bounds;

--------------040901020700020509000906--