You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avalon.apache.org by Gregory Steuck <gr...@nest.cx> on 2002/03/05 04:15:36 UTC
VariableSizeBuffer growth bug (was: DefaultQueue not really threadsafe?)
>>>>> "Peter" == Peter Royal <pr...@managingpartners.com> writes:
Peter> I've attached a simple test to illustrate the problem. It
Peter> takes a single
Peter> argument which is the number of elements to queue up,
Peter> 1000000 is a good start to expose the error.
As far as I can tell there is a bug in method add in rev 1.2-1.4 of
VariableSizeBuffer. The way it handles array growth is not right when
(m_tail < m_head). Buffer ends up containing a bunch of nulls that
were never put there. I will try to produce a test case and a fix
tomorrow unless somebody beats me to it...
And I could also reproduce Peter's results with current CVS code.
Starting test
Test complete
Enqueue: 1000000
Dequeue: 950846
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: VariableSizeBuffer growth bug (was: DefaultQueue not really threadsafe?)
Posted by Berin Loritsch <bl...@apache.org>.
Marcus Crafter wrote:
> On Tue, Mar 05, 2002 at 08:55:04AM -0500, Berin Loritsch wrote:
>
>>Gregory Steuck wrote:
>>
>>>>>>>>"Peter" == Peter Royal <pr...@managingpartners.com> writes:
>>>>>>>>
>>>>>>>>
>>>As far as I can tell there is a bug in method add in rev 1.2-1.4 of
>>>VariableSizeBuffer. The way it handles array growth is not right when
>>>(m_tail < m_head). Buffer ends up containing a bunch of nulls that
>>>were never put there. I will try to produce a test case and a fix
>>>tomorrow unless somebody beats me to it...
>>>
>
> It definitely doesn't look threadsafe:
The buffer classes themselves are not ThreadSafe. It was never the
intention for them to be that way. It is the Queue that must be
ThreadSafe. The Queue is supposed to synchronize on the buffer for
each add and remove operation.
> from VariableSizeBuffer.add():
>
> if( size() + 1 >= m_buffer.length )
> {
> Object[] tmp = new Object[ ( (m_buffer.length - 1) * 2) + 1
> ];
>
> for ( int i = 0; i < m_buffer.length; i++ )
> {
> tmp[i] = m_buffer[i];
> m_buffer[i] = null;
> }
>
> m_buffer = tmp;
> }
>
> If 2 threads add objects at the same time causing the internal
> buffer to grow, it could cause the grow code to happen in each thread.
Entirely possible, but that means that the Queue needs to manage the
synchronization on the buffer.
> What you are probably seeing is one thread set m_buffer[i] to null a
> few iterations ahead of the other thread, with it copying those nulls
> across into the new Object array which replaces m_buffer.
>
> Perhaps this growth code should be considered a critical area if
> the class should be threadsafe ?
The buffer is not intended to be ThreadSafe. Synchronization should be
in the Queue.
--
"They that give up essential liberty to obtain a little temporary safety
deserve neither liberty nor safety."
- Benjamin Franklin
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: VariableSizeBuffer growth bug (was: DefaultQueue not really threadsafe?)
Posted by Marcus Crafter <cr...@fztig938.bank.dresdner.net>.
On Tue, Mar 05, 2002 at 08:55:04AM -0500, Berin Loritsch wrote:
> Gregory Steuck wrote:
> >>>>>>"Peter" == Peter Royal <pr...@managingpartners.com> writes:
> >>>>>>
> >
> >As far as I can tell there is a bug in method add in rev 1.2-1.4 of
> >VariableSizeBuffer. The way it handles array growth is not right when
> >(m_tail < m_head). Buffer ends up containing a bunch of nulls that
> >were never put there. I will try to produce a test case and a fix
> >tomorrow unless somebody beats me to it...
It definitely doesn't look threadsafe:
from VariableSizeBuffer.add():
if( size() + 1 >= m_buffer.length )
{
Object[] tmp = new Object[ ( (m_buffer.length - 1) * 2) + 1
];
for ( int i = 0; i < m_buffer.length; i++ )
{
tmp[i] = m_buffer[i];
m_buffer[i] = null;
}
m_buffer = tmp;
}
If 2 threads add objects at the same time causing the internal
buffer to grow, it could cause the grow code to happen in each thread.
What you are probably seeing is one thread set m_buffer[i] to null a
few iterations ahead of the other thread, with it copying those nulls
across into the new Object array which replaces m_buffer.
Perhaps this growth code should be considered a critical area if
the class should be threadsafe ?
Cheers,
Marcus
--
.....
,,$$$$$$$$$, Marcus Crafter
;$' '$$$$: Computer Systems Engineer
$: $$$$: ManageSoft GmbH
$ o_)$$$: 82-84 Mainzer Landstrasse
;$, _/\ &&:' 60327 Frankfurt Germany
' /( &&&
\_&&&&'
&&&&.
&&&&&&&:
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Test and patch for VariableSizeBuffer growth bug
Posted by Gregory Steuck <gr...@nest.cx>.
>>>>> "Berin" == Berin Loritsch <bl...@apache.org> writes:
Berin> Question: Do you have actual multiprocessor systems to test
Berin> on? This is one of my main limitations.
Nope, just plain single CPU Linux 2.4 box with jdk 1.4.
Attached is the promissed test case and a patch for
VariableSizeBuffer. With the new code QueueTest works fine. But I
share Leo's concern about DefaultQueue implementation. I will send a
separate email about it.
Re: VariableSizeBuffer growth bug (was: DefaultQueue not really threadsafe?)
Posted by Berin Loritsch <bl...@apache.org>.
Gregory Steuck wrote:
>>>>>>"Peter" == Peter Royal <pr...@managingpartners.com> writes:
>>>>>>
>
> Peter> I've attached a simple test to illustrate the problem. It
> Peter> takes a single
>
> Peter> argument which is the number of elements to queue up,
> Peter> 1000000 is a good start to expose the error.
>
> As far as I can tell there is a bug in method add in rev 1.2-1.4 of
> VariableSizeBuffer. The way it handles array growth is not right when
> (m_tail < m_head). Buffer ends up containing a bunch of nulls that
> were never put there. I will try to produce a test case and a fix
> tomorrow unless somebody beats me to it...
>
> And I could also reproduce Peter's results with current CVS code.
>
> Starting test
> Test complete
> Enqueue: 1000000
> Dequeue: 950846
Question: Do you have actual multiprocessor systems to test on?
This is one of my main limitations.
My machine passes the test--1000000 in 1000000 out.
I am running JDK 1.4
--
"They that give up essential liberty to obtain a little temporary safety
deserve neither liberty nor safety."
- Benjamin Franklin
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>