Hi Manuel,
that's correct - it looks like there's leakage in trunk. Though
before changing it back - I need to understand the implications of
doing so
thanks for spotting this btw!
cheers,
Rob
On Jul 10, 2007, at 8:02 AM, Manuel Teira wrote:
Rob Davies escribió:
hi Manuel,
on the face of it - this does look like an issue was introduced by
the change in trunk you mentioned. I think the code was changed
to help some concurrency issues. However, the broker has changed
considerably since then, so I'm not sure it still applies. The
best thing to do would be to apply your patch + rollback the
change in AbstractBroker you highlighted, and see if anything
breaks ;)
Well, perhaps I didn't explain it correctly. The changes I was
talking about (destinations created inside the TopicRegion and not
being created from top) are only applied in the trunk (509706),
not in the 4.1 branch.
The fact is that once I found the problem, and knowing that the
trunk was not suffering the leakage problem I'm behind, I wanted to
know the reason, and found these differences.
What I would like to know is how is it now (now=trunk) supposed to
work. Changes in 509706 are, if I understand correctly, avoiding
the auto-created topics to be exposed in the RegionBroker. So,
when the AdvisoryBroker tries to delete them, propagating the
removeDestination to the chained brokers, it won't find them. True?
Regards.
cheers,
Rob
On Jul 9, 2007, at 1:57 PM, Manuel Teira wrote:
And even worse, creating the destination in this way (inside the
TopicRegion) is not going to make the RegionBroker aware of it.
Since the AdvisoryBroker is relaying in the broker chain to try
to delete the consumer and producer advisory topics when a
destination is deleted, how and when are they supposed to be
deleted in the trunk code?
Regards.
Manuel Teira escribió:
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.
Rob Davies
'Go further faster with Apache Camel!'
http://rajdavies.blogspot.com/
