Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix error with DNS resolution breaking endpoint iterator #1393

Merged
merged 2 commits into from
Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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());
}
}
}