[
https://issues.apache.org/jira/browse/QPID-6028?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alex Rudyy reopened QPID-6028:
------------------------------
Reopening this JIRA as I think that changes made in {{FanoutExchangeImpl}} make
it susceptible to memory leaks.
If binding does not have filter arguments, it is added into {{_queue}} and
{{_unfilteredQueues }} (of {{FanoutExchangeImpl$BindingSet}}) (If binding is
added multiple times, the counter in _queue is incremented)
If binding has filter argument, it is added into {{_filteredQueues}} and
{{_filteredBindings}} (of {{FanoutExchangeImpl$BindingSet}}).
Thus imagine the following scenario, when queue bindings are added to a fanout
exchange:
1) add binding with filter and routing key set to null (binding is added into
{{_filteredQueues}} and {{_filteredBindings}})
2) add binding with filter and routing key set to 'a' (binding is added into
{{_filteredBindings}} ; {{_filteredQueue}} is not modified because queue is
already there)
3) add binding without filter and routing key set to null (binding with null
routing key is removed from {{_filteredBindings}} as part of
{{Exchange#repaceBinding}} and added into {{_queues}} and {{_unfilteredQueue}}
({{_filteredQueues}} still has entry for a queue for routing key 'a'))
4) add binding without filter and routing key set to null again (binding with
null routing key is removed from {{_unfilteredQueues}} and {{_queues }}as part
of {{Exchange#repaceBinding}} but it is addeed into {{_filteredQueues}} second
time (!!!) as {{_filteredBindings}} has binding for a routing key 'a'; it is
re-added to {{_unfilteredQueues}} and {{_queues}} as part of
{{Exchange#repaceBinding}}.
5) Repeating step 4 in a loop multiple times would result in adding into
{{_filteredQueues}} a queue entry every time (line 203) which might eventually
result in OOM.
Here is a second scenario when leak is possible
1) add binding with filter and routing key set to 'a' (binding is added into
{{_filteredQueues}} and {{_filteredBindings}})
1) add binding without filter and routing key set to null (added into
{{_queues}} and {{_unfilteredQueues}}. The queue is present in
{{_filteredQueues}} and {{_filteredBindings}} due to step 1)
2) add binding with filter and routing key set to 'b' (binding is added into
{{_filteredQueues}} second time as it is present in {{_unfilteredQueues}});
binding is added into {{_filteredBindings}}
3) Repeating step 2 in a loop multiple times would result in adding into
{{_filteredQueues}} a queue entry every time (line 91) which might eventually
result in OOM.
I think that implementation of {{FanoutExchangeImpl$BindingSet}} does not
really need {{_filteredQueues}} and {{_unfilteredQueues}}. These fields look
redundant to me.
Map {{_queues}} already containa unfiltered queues, thus
{{_unfilteredQueues.contains}} can be replaced with call to
{{_queues.containsKey}}.
Map {{_filteredBindings}} contains filtered queues, thus
{{_filteredQueues.contains}} can be replaced with call to
{{_filteredBindings.containsKey}}.
I think that {{_filteredBindings}} can be simplified and replaced with
{{Map<MessageDestination, FilterManager> _filteredBindings}}. Every new filter
can be added into {{FilterManage}}r using {{FilterManager#addFilter(String
name,MessageFilter filter)}} similar to {{DirectExchangeImp}} where we can pass
binding key as a name .
> [Java Broker] Configured model objects should have only one parent
> ------------------------------------------------------------------
>
> Key: QPID-6028
> URL: https://issues.apache.org/jira/browse/QPID-6028
> Project: Qpid
> Issue Type: Improvement
> Components: Java Broker
> Reporter: Rob Godfrey
> Fix For: qpid-java-broker-7.0.0
>
>
> Currently it is possible for a configured object to be defined as having
> multiple parents (of different classes)
> Thus a binding has a "queue" parent and an "exchange" parent, a consumer has
> a "session" parent and a "queue" parent, a virtualhostalias has a "port"
> parent and a virtialhost parent.
> This design should be changed.
> h5. Bindings
> Bindings should have a single exchange parent with the queue being an
> attribute (note that this probably also requires adding binding-key as an
> attribute and setting the name to queue/binding-key or some such.
> Exchange-wide validation on the binding-key will be required)
> h5. Consumers
> Since in AMQP 1.0 a link endpoint may outlive the session that created it, it
> makes sense for the parent of the Consumer to be the Queue. The Session will
> be a (derived) attribute of the Consumer.
> h5. VirtualhostAliases
> The primary parent of the virtualhostalias should be the (amqp) port. The
> virtualHost should be an attribute. On creating a VirtualHost we should
> offer to create an alias for the virtual host on all existing ports with the
> host name as the alias. On creating a port we should offer to create aliases
> for each of the existing virtualhosts.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
