Mailing-List: contact activemq-dev-help@geronimo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: activemq-dev@geronimo.apache.org
Received-SPF: pass (asf.osuosl.org: local policy)
In-Reply-To: <20051223094801.34751.qmail@minotaur.apache.org>
References: <20051223094801.34751.qmail@minotaur.apache.org>
Mime-Version: 1.0 (Apple Message framework v746.2)
Content-Type: text/plain; charset=US-ASCII; delsp=yes; format=flowed
Message-Id: <4A89BE96-080B-4E79-B34E-70E470A8C998@hiramchirino.com>
Cc: activemq-commits@geronimo.apache.org
Content-Transfer-Encoding: 7bit
From: Hiram Chirino <hiram@hiramchirino.com>
Subject: Re: svn commit: r358785 -
 /incubator/activemq/trunk/activemq-core/src/main/java/org/activemq/broker/region/PrefetchSubscription.java
Date: Fri, 23 Dec 2005 12:37:08 -0500
To: activemq-dev@geronimo.apache.org

Hi Adrian,

doing a getMessage() before incrementReferenceCount() is dangerous  
since the message could have been swapped out and the call to  
getMessage() will return null.
Yes, I know there is a null check to see if the message is null, but  
that should only happen if the message was expired.  Right now I  
think it's possible that we are going to have cases of where messages  
get swapped out and this code is going to think that the message has  
been expired.

Regards,
Hiram


On Dec 23, 2005, at 4:48 AM, aco@apache.org wrote:

> Author: aco
> Date: Fri Dec 23 01:47:47 2005
> New Revision: 358785
>
> URL: http://svn.apache.org/viewcvs?rev=358785&view=rev
> Log:
> Postpone incrementing of reference count and preload size, only  
> after we are sure that the message will be dispatched by the  
> current subscription. This is to prevent a memory leak type of  
> scenario.
>
> Modified:
>     incubator/activemq/trunk/activemq-core/src/main/java/org/ 
> activemq/broker/region/PrefetchSubscription.java
>
> Modified: incubator/activemq/trunk/activemq-core/src/main/java/org/ 
> activemq/broker/region/PrefetchSubscription.java
> URL: http://svn.apache.org/viewcvs/incubator/activemq/trunk/ 
> activemq-core/src/main/java/org/activemq/broker/region/ 
> PrefetchSubscription.java?rev=358785&r1=358784&r2=358785&view=diff
> ====================================================================== 
> ========
> --- incubator/activemq/trunk/activemq-core/src/main/java/org/ 
> activemq/broker/region/PrefetchSubscription.java (original)
> +++ incubator/activemq/trunk/activemq-core/src/main/java/org/ 
> activemq/broker/region/PrefetchSubscription.java Fri Dec 23  
> 01:47:47 2005
> @@ -239,19 +239,19 @@
>
>      private void dispatch(final MessageReference node) throws  
> IOException {
>
> -        node.incrementReferenceCount();
> -
>          final Message message = node.getMessage();
>          if( message == null ) {
>              return;
> -        }
> -        incrementPreloadSize(node.getMessage().getSize());
> +        }
>
>          // Make sure we can dispatch a message.
>          if( canDispatch(node) ) {
>
>              MessageDispatch md = createMessageDispatch(node,  
> message);
>              dispatched.addLast(node);
> +
> +            node.incrementReferenceCount();
> +            incrementPreloadSize(node.getMessage().getSize());
>
>              if( info.isDispatchAsync() ) {
>                  md.setConsumer(new Runnable(){
>