activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Manuel Teira <mte...@tid.es>
Subject Re: Advisory topic leakages (second theory)
Date Mon, 09 Jul 2007 08:47:41 GMT
Hi again.

I'm afraid that the lookup code in AbstractRegion has being changed in 
the trunk (I was looking at 4.1 branch). Basically, instead of calling 
context.getBroker().addDestination(context,destination) to create the 
new destination, addDestination(context, destination) is used. This way, 
the advisory topic won't be created from top, but what happens with the 
advise in the code?

                if(autoCreateDestinations){
                    // Try to auto create the destination... re-invoke 
broker from the
                    // top so that the proper security checks are performed.
                    try {
                        dest = addDestination(context, destination);
                        
//context.getBroker().addDestination(context,destination);
                    }
I suppose that the assumption is no longer true.

Also, the way to change this code (only commenting out the old one) 
makes me think about a not too mature change?

If you can verify that this is the right way to proceed,I will like to 
prepare a patch against 4.1 branch.


Best regards.


Manuel Teira escribió:
> Hello again. Digging into the problem I've found another thing related 
> with an asymmetry in the way an advisory topic  is created and destroyed.
>
> I'm analizying the way the Consumer and Producer advisory topics for 
> temporary queues are created and destroyed:
>
> An advisory topic is actually created when the AdvisoryBroker 
> fireAdvisory method is eventually sending the message. This is 
> happening in AbstractRegion lookup method, as the advisory topic 
> doesn't exist yet:
>
>    protected Destination lookup(ConnectionContext context, 
> ActiveMQDestination destination) throws Exception {
>        synchronized(destinationsMutex){
>            Destination dest=(Destination) destinations.get(destination);
>            if(dest==null){
>                if(autoCreateDestinations){
>                    // Try to auto create the destination... re-invoke 
> broker from the
>                    // top so that the proper security checks are 
> performed.
>                    
> context.getBroker().addDestination(context,destination);
>                    // We should now have the dest created.
>                    dest=(Destination) destinations.get(destination);
>                }
>                if(dest==null){
>                    throw new JMSException("The destination 
> "+destination+" does not exist.");
>                }
>            }
>            return dest;
>        }
>    }
>
> Hence, the whole Broker chain is called to create a destination 
> (context.getBroker().addDestination), this, in a common environment, 
> involves calling:
>
> MutableBrokerFilter.addDestination - Just pass the request to the next 
> chained BrokerFilter
> [
>   Here the configured plugins
> }
> CompositeDestinationBroker. No implementation, so it passes the 
> request to the next chained object.
> AdvisoryBroker. Fires an advisory to the destination advisory topic, 
> and adds the destination to its own destinations map.
> TransactionBroker.  No implementation, passes the request to the next 
> chained object.
> RegionBroker. Delegates in the TopicRegion.addDestination to create 
> the given advisory topic.
>
> On the other way, this advisory topic is destroyed when the advised 
> destination is removed, in AdvisoryTopic. removeDestinationInfo. But 
> here, the way to do it is:
>
>    public void removeDestinationInfo(ConnectionContext context, 
> DestinationInfo destInfo) throws Exception{
>        next.removeDestinationInfo(context, destInfo);
>        DestinationInfo info = (DestinationInfo) 
> destinations.remove(destInfo.getDestination());
>
>        if( info !=null ) {
>            info.setDestination(destInfo.getDestination());
>            info.setOperationType(DestinationInfo.REMOVE_OPERATION_TYPE);
>            ActiveMQTopic topic = 
> AdvisorySupport.getDestinationAdvisoryTopic(destInfo.getDestination());
>            fireAdvisory(context, topic, info);
>            try {
>                next.removeDestination(context, 
> AdvisorySupport.getConsumerAdvisoryTopic(info.getDestination()), -1);
>            } catch (Exception expectedIfDestinationDidNotExistYet) {
>            }
>            try {
>                next.removeDestination(context, 
> AdvisorySupport.getProducerAdvisoryTopic(info.getDestination()), -1);
>            } catch (Exception expectedIfDestinationDidNotExistYet) {
>            }
>        }
>
>    }
>
>
> So, only the next chained broker components to AdvisoryBroker are 
> called to remove the consumer and producer advisory topics. This way 
> to proceed suggests me two problems:
>
> 1.-The advisory broker itself is not aware of the deletion of those 
> topics (remember that it had registered them when the whole broker 
> chain was called to create the topic). I think that this is the 
> leakage I'm suffering.
> 2.-Any plugin (or component in the chain preceding the AdvisoryBroker) 
> that could be creating and retaining objects related with these 
> advisory topics won't never be able to release them.
>
> Perhaps this way to proceed could be related with the fix of AMQ-677.
>
> Did I miss anything?
>
> Regards.
>
>
>


Mime
View raw message