Skip to content

Commit

Permalink
fix error with DNS resolution breaking endpoint iterator (#1393)
Browse files Browse the repository at this point in the history
* re-implement GetEnumerator in EndPointCollection to be a bit more forgiving to concurrent changes during iteration

* poke release notes
  • Loading branch information
mgravell authored Mar 18, 2020
1 parent 3f0c2b8 commit 748f1fa
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
1 change: 1 addition & 0 deletions docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- fix: `ScriptEvaluateAsync` keyspace isolation (#1377 via gliljas)
- fix: F# compatibility enhancements (#1386)
- fix: improved `ScriptResult` null support (#1392)
- fix: error with DNS resolution breaking endpoint iterator (#1393)
- tests: better docker support for tests (#1389 via ejsmith; #1391)
- tests: general test improvements (#1183, #1385, #1384)

Expand Down
19 changes: 18 additions & 1 deletion src/StackExchange.Redis/EndPointCollection.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Net;
Expand All @@ -8,7 +9,7 @@ namespace StackExchange.Redis
/// <summary>
/// A list of endpoints
/// </summary>
public sealed class EndPointCollection : Collection<EndPoint>
public sealed class EndPointCollection : Collection<EndPoint>, IEnumerable, IEnumerable<EndPoint>
{
/// <summary>
/// Create a new EndPointCollection
Expand Down Expand Up @@ -109,5 +110,21 @@ internal void SetDefaultPorts(int defaultPort)
}
}
}

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
IEnumerator<EndPoint> IEnumerable<EndPoint>.GetEnumerator() => GetEnumerator();

/// <inheritdoc/>
public new IEnumerator<EndPoint> GetEnumerator()
{
// this does *not* need to handle all threading scenarios; but we do
// want it to at least allow overwrites of existing endpoints without
// breaking the enumerator; in particular, this avoids a problem where
// ResolveEndPointsAsync swaps the addresses on us
for (int i = 0; i < Count; i++)
{
yield return this[i];
}
}
}
}
18 changes: 18 additions & 0 deletions tests/StackExchange.Redis.Tests/Config.cs
Original file line number Diff line number Diff line change
Expand Up @@ -385,5 +385,23 @@ public async Task TestAutomaticHeartbeat()
}
}
}

[Fact]
public void EndpointIteratorIsReliableOverChanges()
{
var eps = new EndPointCollection
{
{ IPAddress.Loopback, 7999 },
{ IPAddress.Loopback, 8000 },
};

using var iter = eps.GetEnumerator();
Assert.True(iter.MoveNext());
Assert.Equal(7999, ((IPEndPoint)iter.Current).Port);
eps[1] = new IPEndPoint(IPAddress.Loopback, 8001); // boom
Assert.True(iter.MoveNext());
Assert.Equal(8001, ((IPEndPoint)iter.Current).Port);
Assert.False(iter.MoveNext());
}
}
}

0 comments on commit 748f1fa

Please sign in to comment.