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.

Sign up for GitHub

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 #2576: PhysicalConnection: Better shutdown handling #2629

Merged
merged 3 commits into from
Jan 11, 2024
Merged

Fix #2576: PhysicalConnection: Better shutdown handling #2629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
408a009
PhysicalConnection: Better shutdown handling re: null refs
NickCraver Jan 11, 2024
af70390
Align WriteSha1AsHex
NickCraver Jan 11, 2024
32de975
Add release notes
NickCraver Jan 11, 2024
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Current package versions:

- Fix [#2619](https://github.com/StackExchange/StackExchange.Redis/issues/2619): Type-forward `IsExternalInit` to support down-level TFMs ([#2621 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2621))
- `InternalsVisibleTo` `PublicKey` enhancements([#2623 by WeihanLi](https://github.com/StackExchange/StackExchange.Redis/pull/2623))
- Fix [#2576](https://github.com/StackExchange/StackExchange.Redis/issues/2576): Prevent `NullReferenceException` during shutdown of connections ([#2629 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2629))

## 2.7.10

Expand Down
54 changes: 39 additions & 15 deletions src/StackExchange.Redis/PhysicalConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,39 +790,44 @@ internal void Write(in RedisKey key)
var val = key.KeyValue;
if (val is string s)
{
WriteUnifiedPrefixedString(_ioPipe!.Output, key.KeyPrefix, s);
WriteUnifiedPrefixedString(_ioPipe?.Output, key.KeyPrefix, s);
}
else
{
WriteUnifiedPrefixedBlob(_ioPipe!.Output, key.KeyPrefix, (byte[]?)val);
WriteUnifiedPrefixedBlob(_ioPipe?.Output, key.KeyPrefix, (byte[]?)val);
}
}

internal void Write(in RedisChannel channel)
=> WriteUnifiedPrefixedBlob(_ioPipe!.Output, ChannelPrefix, channel.Value);
=> WriteUnifiedPrefixedBlob(_ioPipe?.Output, ChannelPrefix, channel.Value);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void WriteBulkString(in RedisValue value)
=> WriteBulkString(value, _ioPipe!.Output);
internal static void WriteBulkString(in RedisValue value, PipeWriter output)
=> WriteBulkString(value, _ioPipe?.Output);
internal static void WriteBulkString(in RedisValue value, PipeWriter? maybeNullWriter)
{
if (maybeNullWriter is not PipeWriter writer)
{
return; // Prevent null refs during disposal
}

switch (value.Type)
{
case RedisValue.StorageType.Null:
WriteUnifiedBlob(output, (byte[]?)null);
WriteUnifiedBlob(writer, (byte[]?)null);
break;
case RedisValue.StorageType.Int64:
WriteUnifiedInt64(output, value.OverlappedValueInt64);
WriteUnifiedInt64(writer, value.OverlappedValueInt64);
break;
case RedisValue.StorageType.UInt64:
WriteUnifiedUInt64(output, value.OverlappedValueUInt64);
WriteUnifiedUInt64(writer, value.OverlappedValueUInt64);
break;
case RedisValue.StorageType.Double: // use string
case RedisValue.StorageType.String:
WriteUnifiedPrefixedString(output, null, (string?)value);
WriteUnifiedPrefixedString(writer, null, (string?)value);
break;
case RedisValue.StorageType.Raw:
WriteUnifiedSpan(output, ((ReadOnlyMemory<byte>)value).Span);
WriteUnifiedSpan(writer, ((ReadOnlyMemory<byte>)value).Span);
break;
default:
throw new InvalidOperationException($"Unexpected {value.Type} value: '{value}'");
Expand All @@ -833,6 +838,11 @@ internal static void WriteBulkString(in RedisValue value, PipeWriter output)

internal void WriteHeader(RedisCommand command, int arguments, CommandBytes commandBytes = default)
{
if (_ioPipe?.Output is not PipeWriter writer)
{
return; // Prevent null refs during disposal
}

var bridge = BridgeCouldBeNull ?? throw new ObjectDisposedException(ToString());

if (command == RedisCommand.UNKNOWN)
Expand All @@ -856,14 +866,14 @@ internal void WriteHeader(RedisCommand command, int arguments, CommandBytes comm
// *{argCount}\r\n = 3 + MaxInt32TextLen
// ${cmd-len}\r\n = 3 + MaxInt32TextLen
// {cmd}\r\n = 2 + commandBytes.Length
var span = _ioPipe!.Output.GetSpan(commandBytes.Length + 8 + Format.MaxInt32TextLen + Format.MaxInt32TextLen);
var span = writer.GetSpan(commandBytes.Length + 8 + Format.MaxInt32TextLen + Format.MaxInt32TextLen);
span[0] = (byte)'*';

int offset = WriteRaw(span, arguments + 1, offset: 1);

offset = AppendToSpanCommand(span, commandBytes, offset: offset);

_ioPipe.Output.Advance(offset);
writer.Advance(offset);
}

internal void RecordQuit() // don't blame redis if we fired the first shot
Expand Down Expand Up @@ -1116,7 +1126,11 @@ private static int AppendToSpan(Span<byte> span, ReadOnlySpan<byte> value, int o

internal void WriteSha1AsHex(byte[] value)
{
var writer = _ioPipe!.Output;
if (_ioPipe?.Output is not PipeWriter writer)
{
return; // Prevent null refs during disposal
}

if (value == null)
{
writer.Write(NullBulkString.Span);
Expand Down Expand Up @@ -1156,8 +1170,13 @@ internal static byte ToHexNibble(int value)
return value < 10 ? (byte)('0' + value) : (byte)('a' - 10 + value);
}

internal static void WriteUnifiedPrefixedString(PipeWriter writer, byte[]? prefix, string? value)
internal static void WriteUnifiedPrefixedString(PipeWriter? maybeNullWriter, byte[]? prefix, string? value)
{
if (maybeNullWriter is not PipeWriter writer)
{
return; // Prevent null refs during disposal
}

if (value == null)
{
// special case
Expand Down Expand Up @@ -1259,8 +1278,13 @@ internal static unsafe void WriteRaw(PipeWriter writer, string value, int expect
}
}

private static void WriteUnifiedPrefixedBlob(PipeWriter writer, byte[]? prefix, byte[]? value)
private static void WriteUnifiedPrefixedBlob(PipeWriter? maybeNullWriter, byte[]? prefix, byte[]? value)
{
if (maybeNullWriter is not PipeWriter writer)
{
return; // Prevent null refs during disposal
}

// ${total-len}\r\n
// {prefix}{value}\r\n
if (prefix == null || prefix.Length == 0 || value == null)
Expand Down
Loading