[jboss-user] [JCA/JBoss] - Bug in connection validation code

[bortx]

Hi,

We have a production environment with jboss 4.2.3. It has been working successfully until now, when we are experiencing network issues. TCP connections are broken very often. Also we noticed that a great number of tcp connections in a ESTABLISHED status were beginning to increase out of control. We have a pool of 9 max connections and we had 3 hundred of ESTABLISHED connections and rising. So we had a look at the code and noticed a bug in connection validation, in org.jboss.resource.connectionmanager.InternalManagedConnectionPool class  validateConnections method.


  |         boolean destroyed = false;
  | 
  |          try
  |          {
  | 
  |             while (true)
  |             {
  | 
  |                ConnectionListener cl = null;
  | 
  |                synchronized (cls)
  |                {
  |                   if (cls.size() == 0)
  |                   {
  |                      break;
  |                   }
  | 
  |                   cl = removeForFrequencyCheck();
  | 
  |                }
  | 
  |                if (cl == null)
  |                {
  |                   break;
  |                }
  | 
  |                try
  |                {
  | 
  |                   Set candidateSet = Collections.singleton(cl.getManagedConnection());
  | 
  |                   if (mcf instanceof ValidatingManagedConnectionFactory)
  |                   {
  |                      ValidatingManagedConnectionFactory vcf = (ValidatingManagedConnectionFactory) mcf;
  |                      candidateSet = vcf.getInvalidConnections(candidateSet);
  | 
  |                      if (candidateSet != null && candidateSet.size() > 0)
  |                      {
  | 
  |                         if (cl.getState() != ConnectionListener.DESTROY)
  |                         {
  |                            doDestroy(cl);
  |                            destroyed = true;
  |                         }
  |                      }
  | 
  |                   }
  |                   else
  |                   {
  |                      log.warn("warning: background validation was specified with a non compliant ManagedConnectionFactory interface.");
  |                   }
  | 
  |                }
  |                finally
  |                {
  |                   if(!destroyed)
  |                   {
  |                      synchronized (cls)
  |                      {
  |                         returnForFrequencyCheck(cl);
  |                      }                     
  |                   }
  | 
  |                }
  | 
  |             }
  | 
  |          }
  |          finally
  |          {
  |             permits.release();
  | 
  |             if (destroyed && shutdown.get() == false && poolParams.minSize > 0)
  |             {
  |                PoolFiller.fillPool(this);
  |             }
  | 
  |          }
  | 

What happens when you have a set of 3 connections in the pool, and when you run this validation code, the fist one is not valid and the following 2 are valid? I will tell you. Those 2 valid connections remain established but they don't return to the pool, resulting that you have 2 established connections out of control. Besides, counters don't get updated and the resource adapter turns into an inconsistent status. All of this is because you don't update destroyed variable to false on every iteration. I think it is not necesary to explain this more in detail...

So I have developed the following simple solution to fix this issue:


  |          boolean anyDestroyed = false;
  | 
  |          try {
  | 
  |             while (true) {
  | 
  |                boolean destroyed = false;
  | 
  |                ConnectionListener cl = null;
  | 
  |                synchronized (cls) {
  |                   if (cls.size() == 0) {
  |                      break;
  |                   }
  | 
  |                   cl = removeForFrequencyCheck();
  | 
  |                }
  | 
  |                if (cl == null) {
  |                   break;
  |                }
  | 
  |                try {
  | 
  |                   Set candidateSet = Collections.singleton(cl.getManagedConnection());
  | 
  |                   if (mcf instanceof ValidatingManagedConnectionFactory) {
  |                      ValidatingManagedConnectionFactory vcf = (ValidatingManagedConnectionFactory) mcf;
  |                      candidateSet = vcf.getInvalidConnections(candidateSet);
  | 
  |                      if (candidateSet != null && candidateSet.size() > 0) {
  | 
  |                         if (cl.getState() != ConnectionListener.DESTROY) {
  |                            doDestroy(cl);
  |                            destroyed = true;
  |                            anyDestroyed = true;
  |                         }
  |                      }
  | 
  |                   } else {
  |                      log.warn("warning: background validation was specified with a non compliant ManagedConnectionFactory interface.");
  |                   }
  | 
  |                } finally {
  |                   if (!destroyed) {
  |                      synchronized (cls) {
  |                         returnForFrequencyCheck(cl);
  |                      }
  |                   }
  | 
  |                }
  | 
  |             }
  | 
  |          } finally {
  |             permits.release();
  | 
  |             if (anyDestroyed && shutdown.get() == false && poolParams.minSize > 0) {
  |                PoolFiller.fillPool(this);
  |             }
  | 
  |          }
  | 

I've had a look at the jira and haven't found this bug as reported. If it has been already fixed i apologize for making you loose your time, if not, it would be so kind of you to send me one of those jboss merchandising caps as I have helped you to find several jca bugs so far... ;) Actually, I think this bug is so big that I can't understand how nobody has noticed it before...

View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4250999#4250999

Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=4250999