Skip to content

Commit 1a771e9

Browse files
jimmym715Jim Mallmann
authored andcommitted
In AutorecoveringModel, each time the QueueBind method is called, a new RecordedQueueBinding object is created and passed to the RecordBinding method of the AutorecoveringConnection class.
That RecordBinding method of the AutorecoveringConnection class expects a single parameter of type RecordedBinding, which RecordedQueueBinding extends, and that RecordedBinding is added to the protected m_recordedBindings variable that's declared as type HashSet<RecordedBinding>. Ideally, a HashSet<T> ensures that it keeps no duplicate instances of T, and, if one looks at the change made in the commit of 2014-09-30 with SHA-1 of 5db2f44, one can see that the reason for using a HashSet for m_recordedBindings is, indeed, to ensure that no duplicate bindings are retained. In that commit, in the AutorecoveringConnection class, the type for m_recordedBindings was changed from List<RecordedBinding> to HashSet<RecordedBinding>, and the RecordBinding method was changed from the following: public void RecordBinding(RecordedBinding rb) { lock(this.m_recordedEntitiesLock) { // TODO: this operation is O(n) if(!m_recordedBindings.Contains(rb)) { m_recordedBindings.Add(rb); } } } to this: public void RecordBinding(RecordedBinding rb) { lock(this.m_recordedEntitiesLock) { m_recordedBindings.Add(rb); } } Notice that the check for whether or not the RecordedBinding is in the collection has been removed. Why? Because, again, using HashSet<RecordedBinding> should ensure that the collection contains no duplicates of RecordedBinding. However, for that to be true, one must override both the GetHashCode() method and the Equals(object obj) method of RecordedBinding.cs Unfortunately, while RecordedBinding had overridden the GetHashCode() method, the only custom Equals method defined had the following signature: bool Equals(RecordedBinding other) Thus, because the Equals(object obj) method was not overridden, every call to QueueBind has resulted in a new object being added to m_recordedBindings in AutorecoveringConnection, and when one needs to *guarantee* that the Queue binding is in place before publishing each message, with messages being published every second, one can see that this memory leak will cause problems quickly.
1 parent c800e80 commit 1a771e9

File tree

1 file changed

+15
-7
lines changed

1 file changed

+15
-7
lines changed

projects/client/RabbitMQ.Client/src/client/impl/RecordedBinding.cs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,23 +53,31 @@ public RecordedBinding(AutorecoveringModel model) : base(model)
5353
public string Destination { get; set; }
5454
public string RoutingKey { get; protected set; }
5555
public string Source { get; protected set; }
56+
57+
public bool Equals(RecordedBinding other)
58+
{
59+
return other != null &&
60+
(Source.Equals(other.Source)) &&
61+
(Destination.Equals(other.Destination)) &&
62+
(RoutingKey.Equals(other.RoutingKey)) &&
63+
(Arguments == other.Arguments);
64+
}
5665

57-
public bool Equals(RecordedBinding other)
66+
public override bool Equals(object obj)
5867
{
59-
if (ReferenceEquals(other, null))
68+
if (ReferenceEquals(obj, null))
6069
{
6170
return false;
6271
}
6372

64-
if (ReferenceEquals(this, other))
73+
if (ReferenceEquals(this, obj))
6574
{
6675
return true;
6776
}
6877

69-
return (Source.Equals(other.Source)) &&
70-
(Destination.Equals(other.Destination)) &&
71-
(RoutingKey.Equals(other.RoutingKey)) &&
72-
(Arguments == other.Arguments);
78+
var other = obj as RecordedBinding;
79+
80+
return Equals(other);
7381
}
7482

7583
public override int GetHashCode()

0 commit comments

Comments
 (0)