Alex Rudyy created QPID-7728:
--------------------------------
Summary: [Java Broker] Existing implementation of fanout exchange
is susceptible to memory leaks
Key: QPID-7728
URL: https://issues.apache.org/jira/browse/QPID-7728
Project: Qpid
Issue Type: Bug
Components: Java Broker
Affects Versions: qpid-java-broker-7.0.0
Reporter: Alex Rudyy
An implementation of fanout exchange {{FanoutExchangeImpl}} is susceptible to
memory leaks. The scenarios when memory leaks can occur are described below.
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 .
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
