cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Santiago Gala <sg...@hisitech.com>
Subject Re: [PATCH] Re: [C2][Xalan2] Xalan2J problems under heavy load using Apache JMeter
Date Fri, 16 Mar 2001 11:48:05 GMT
Berin Loritsch wrote:

>>>> The synchronization in org.apache.avalon.util.JdbcConnectionPool is
>>>> broken. I was about to patch it, but a major code reorganization is
>>>> required, so I preferred to ask first.
>>>> 
>>> 
>> In this connection pool there are serious race conditions (I have a SMP
>> box to see these things happen before my customers :-).
> 
> 
> Do you know where exactly?  I was thinking about changing the logic to a
> Semaphore based type of pool.  That will fix allot of the race conditions.

First, init() should be synchronized unless you can ensure that no 
get()/put() activity will go on before init() returns. This is not 
important (I hope).

The big issues come in get(). Between the moment you close the first 
synchronized( m_ready ) and the  moment you open it again, m_ready can 
change! Ant the same goes on. The operation should be completely atomic.
Also, there is a type synchronized( this ) which would do nothing, as it 
is not guarding against any other thing in the same class.

 
I would reimplement it completely, locking the whole set of operations. 
A good way to start is to change public methods init(), get(), put(), 
dispose() to synchronized methods and wrapping the while in run() in a 
synchronized( this ).

Later, the performance can be tuned, by ordering operations and using 
synchronized( this ) instead of synchronizing the whole method.

But please, start always with a safe implementation and tune later on.
Also, think of your code as a highway, where drunk threads are running 
into. Unless you have proper traffic lights, they will bump into the 
same places (variables).

This kind of thinking is useful:
(Murphy's law). I have


        synchronized (m_ready)
        {
            size = m_ready.size();
        }

        if( 0 == size )
        { ...

Well, what if a second thread is just doing

(down delow)
                     synchronized (m_ready)
                     {
/*
                         m_ready.wait((end - start)/2);
*/
                         if (m_ready.size() > 0)
                         {
                             obj = (Poolable)m_ready.remove(0);

                             synchronized (m_active)
                             {
                                 m_active.add(obj);
                             }
                         }
                     }
and the first thread exits the first synchronized, and then sleeps. When 
the first thread resumes, size was 1, but the second thread has just 
stealed the remaining object (remove)...

So, every use of a shared resource MUST be serialized atomically. Having 
serialized acces to the size, and then releasing the lock and using the 
size is not valid, as the size can change in between.

Thinking of your code as a highway where threads run is very useful in 
multithreaded programming. Think of variables as lanes or parking space, 
or tickets that a driver picks up and leaves later. Two cars cannot have 
the same ticket at the same time!


> I am the one who originally wrote the code--and I don't have an SMP box
> to test on.  Please, by all means, if you can do it better, do it!

I'll try to have time to do a basic runnable implementation, but full 
optimization should be left for the future (first get it working)

Also, i code like

if( true ) {
     //body
     }

not like

if(true)
{
     //body
}

I hear something around here... :)


---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Mime
View raw message