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

Hotfix: Make HashAlgorithmType.Digest() thread-safe #1411

Merged
merged 1 commit into from
Jul 29, 2021
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
9 changes: 5 additions & 4 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ Version 0.13.1

To be released.

- Fixed `HashAlgorithmType.Digest()` method's bug that it returns an incorrect
digest bytes when it is called by multiple threads at a time. [[#1411]]

[#1411]: https://github.com/planetarium/libplanet/pull/1411


Version 0.13.0
--------------
Expand All @@ -18,10 +23,6 @@ Released on July 28, 2021.
Blocks and actions in preloaded blocks will be rendered if the switch
is set to `true`. [[#1391]]

### Backward-incompatible network protocol changes

### Backward-incompatible storage format changes

### Added APIs

- Added `Transaction<T>.CreateUnsigned()` method. [[#1378]]
Expand Down
39 changes: 39 additions & 0 deletions Libplanet.Tests/HashAlgorithmTypeTest.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Immutable;
using System.Linq;
using System.Security.Cryptography;
using System.Text;
using Xunit;
Expand Down Expand Up @@ -42,6 +43,44 @@ public void Digest()
AssertBytesEqual(expected.ToImmutableArray(), immutableDigest);
}

[Fact]
public void DigestMultipleTimes()
{
HashAlgorithmType sha256 = HashAlgorithmType.Of<SHA256>();
byte[] expected =
{
0x2c, 0xf2, 0x4d, 0xba, 0x5f, 0xb0, 0xa3, 0x0e, 0x26, 0xe8, 0x3b,
0x2a, 0xc5, 0xb9, 0xe2, 0x9e, 0x1b, 0x16, 0x1e, 0x5c, 0x1f, 0xa7,
0x42, 0x5e, 0x73, 0x04, 0x33, 0x62, 0x93, 0x8b, 0x98, 0x24,
};
byte[] input = Encoding.ASCII.GetBytes("hello");
byte[] digest;
for (int i = 0; i < 50; i++)
{
digest = sha256.Digest(input);
AssertBytesEqual(expected, digest);
}
}

[Fact]
public void DigestInParallel()
{
HashAlgorithmType sha256 = HashAlgorithmType.Of<SHA256>();
ImmutableArray<byte> expected = new byte[]
{
0x2c, 0xf2, 0x4d, 0xba, 0x5f, 0xb0, 0xa3, 0x0e, 0x26, 0xe8, 0x3b,
0x2a, 0xc5, 0xb9, 0xe2, 0x9e, 0x1b, 0x16, 0x1e, 0x5c, 0x1f, 0xa7,
0x42, 0x5e, 0x73, 0x04, 0x33, 0x62, 0x93, 0x8b, 0x98, 0x24,
}.ToImmutableArray();
ImmutableArray<byte> input = Encoding.ASCII.GetBytes("hello").ToImmutableArray();
ImmutableArray<byte>[] digests =
Enumerable.Repeat(input, 100).AsParallel().Select(b => sha256.Digest(b)).ToArray();
foreach (ImmutableArray<byte> digest in digests)
{
AssertBytesEqual(expected, digest);
}
}

[Fact]
public void Equality()
{
Expand Down
19 changes: 14 additions & 5 deletions Libplanet/HashAlgorithmType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics.Contracts;
using System.Reflection;
using System.Security.Cryptography;
using System.Threading;

namespace Libplanet
{
Expand All @@ -19,16 +20,24 @@ public sealed class HashAlgorithmType : IEquatable<HashAlgorithmType>
private static readonly ConcurrentDictionary<Type, HashAlgorithmType> _internedObjects
= new ConcurrentDictionary<Type, HashAlgorithmType>();

private readonly HashAlgorithm _instance;
private readonly MethodInfo? _instanceCtor;

private readonly ThreadLocal<HashAlgorithm> _instance;

private HashAlgorithmType(Type type)
{
Type = type;
MethodInfo method = type.GetMethod(nameof(HashAlgorithm.Create), new Type[0])!;
string excMsg =
$"Failed to invoke {type.FullName}.{nameof(HashAlgorithm.Create)}() static method.";
_instance = method?.Invoke(null, new object[0]) as HashAlgorithm
?? throw new InvalidCastException(excMsg);
_instanceCtor = method;
_instance = new ThreadLocal<HashAlgorithm>(() =>
_instanceCtor?.Invoke(null, new object[0]) as HashAlgorithm
?? throw new InvalidCastException(excMsg)
);

// Immediately create an instance to check if the constructor works well:
HashAlgorithm unused = _instance.Value!;
}

/// <summary>
Expand All @@ -42,7 +51,7 @@ private HashAlgorithmType(Type type)
/// The length of bytes of every digest that the <see cref="HashAlgorithmType"/> makes.
/// </summary>
[Pure]
public int DigestSize => _instance.HashSize / 8;
public int DigestSize => _instance.Value!.HashSize / 8;

/// <summary>
/// Checks if two <see cref="HashAlgorithmType"/>s refers to the same
Expand Down Expand Up @@ -89,7 +98,7 @@ public static HashAlgorithmType Of<T>(T? objectToInferType = null)
/// <returns>The hash digest derived from <paramref name="input"/>.</returns>
[Pure]
public byte[] Digest(byte[] input) =>
_instance.ComputeHash(input);
_instance.Value!.ComputeHash(input);

/// <summary>
/// Computes a hash digest of the hash algorithm from the given
Expand Down