Skip to content

Commit

Permalink
PhysicalConnection: Better shutdown handling re: null refs
Browse files Browse the repository at this point in the history 
This adds a bit of null ref handling (few ifs). Fixes #2576.
  • Loading branch information
@NickCraver
NickCraver committed Jan 11, 2024
1 parent ad2e69f commit 408a009
Showing 1 changed file with 63 additions and 41 deletions.
104 changes: 63 additions & 41 deletions src/StackExchange.Redis/PhysicalConnection.cs
View file
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,38 +1126,40 @@ private static int AppendToSpan(Span<byte> span, ReadOnlySpan<byte> value, int o

internal void WriteSha1AsHex(byte[] value)
{
var writer = _ioPipe!.Output;
if (value == null)
if (_ioPipe?.Output is PipeWriter writer)
{
writer.Write(NullBulkString.Span);
}
else if (value.Length == ResultProcessor.ScriptLoadProcessor.Sha1HashLength)
{
// $40\r\n = 5
// {40 bytes}\r\n = 42
if (value == null)
{
writer.Write(NullBulkString.Span);
}
else if (value.Length == ResultProcessor.ScriptLoadProcessor.Sha1HashLength)
{
// $40\r\n = 5
// {40 bytes}\r\n = 42

var span = writer.GetSpan(47);
span[0] = (byte)'$';
span[1] = (byte)'4';
span[2] = (byte)'0';
span[3] = (byte)'\r';
span[4] = (byte)'\n';
var span = writer.GetSpan(47);
span[0] = (byte)'$';
span[1] = (byte)'4';
span[2] = (byte)'0';
span[3] = (byte)'\r';
span[4] = (byte)'\n';

int offset = 5;
for (int i = 0; i < value.Length; i++)
int offset = 5;
for (int i = 0; i < value.Length; i++)
{
var b = value[i];
span[offset++] = ToHexNibble(b >> 4);
span[offset++] = ToHexNibble(b & 15);
}
span[offset++] = (byte)'\r';
span[offset++] = (byte)'\n';

writer.Advance(offset);
}
else
{
var b = value[i];
span[offset++] = ToHexNibble(b >> 4);
span[offset++] = ToHexNibble(b & 15);
throw new InvalidOperationException("Invalid SHA1 length: " + value.Length);
}
span[offset++] = (byte)'\r';
span[offset++] = (byte)'\n';

writer.Advance(offset);
}
else
{
throw new InvalidOperationException("Invalid SHA1 length: " + value.Length);
}
}

Expand All @@ -1156,8 +1168,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 +1276,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

0 comments on commit 408a009

Please sign in to comment.