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

[main] Update dependencies from dotnet/efcore, dotnet/runtime #55339

Merged
merged 26 commits into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f60f963
Update dependencies from https://github.com/dotnet/runtime build 2024…
dotnet-maestro[bot] Apr 24, 2024
ef942a0
Fix build break
javiercn Apr 24, 2024
e10217e
More break fixes
javiercn Apr 24, 2024
277ecba
More break fixes
javiercn Apr 24, 2024
3472997
Try fixing frameworkRef
wtgodbe Apr 24, 2024
80e11cd
Revert "Try fixing frameworkRef"
wtgodbe Apr 24, 2024
9cdbd54
Update dependencies from https://github.com/dotnet/efcore build 20240…
dotnet-maestro[bot] Apr 24, 2024
fd13b17
Revert "Revert "Try fixing frameworkRef""
wtgodbe Apr 25, 2024
67ec656
Update dependencies from https://github.com/dotnet/runtime build 2024…
dotnet-maestro[bot] Apr 25, 2024
0a9c70c
Try fixing condition
wtgodbe Apr 25, 2024
67acd14
Merge remote-tracking branch 'upstream/darc-main-1fe41fe1-d5d1-494c-8…
wtgodbe Apr 25, 2024
f0bc3b7
Try hacky workaround
wtgodbe Apr 25, 2024
7890664
Merge branch 'main' into darc-main-1fe41fe1-d5d1-494c-8d90-7c97802c275f
wtgodbe Apr 25, 2024
07773cd
Update dependencies from https://github.com/dotnet/runtime build 2024…
dotnet-maestro[bot] Apr 26, 2024
84b7c26
Update dependencies from https://github.com/dotnet/runtime build 2024…
dotnet-maestro[bot] Apr 27, 2024
48fbe2d
Update dependencies from https://github.com/dotnet/runtime build 2024…
dotnet-maestro[bot] Apr 28, 2024
0ccca10
Update dependencies from https://github.com/dotnet/runtime build 2024…
dotnet-maestro[bot] Apr 29, 2024
899e787
Update dependencies from https://github.com/dotnet/efcore build 20240…
dotnet-maestro[bot] Apr 29, 2024
f6e2347
Update dependencies from https://github.com/dotnet/efcore build 20240…
dotnet-maestro[bot] Apr 29, 2024
b7f7cf2
Update dependencies from https://github.com/dotnet/efcore build 20240…
dotnet-maestro[bot] Apr 29, 2024
a98ae1f
fix reflection
BrennanConroy Apr 29, 2024
a5dcb85
readonly
BrennanConroy Apr 29, 2024
add61eb
Update dependencies from https://github.com/dotnet/runtime build 2024…
dotnet-maestro[bot] Apr 30, 2024
1462086
remove cache for now
BrennanConroy Apr 30, 2024
4e8dda2
Update dependencies from https://github.com/dotnet/efcore build 20240…
dotnet-maestro[bot] Apr 30, 2024
b75a379
Update dependencies from https://github.com/dotnet/runtime build 2024…
dotnet-maestro[bot] May 1, 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
18 changes: 14 additions & 4 deletions src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -829,12 +829,22 @@ private void LaunchStreams(ConnectionState connectionState, Dictionary<string, o
{
_ = _sendIAsyncStreamItemsMethod
.MakeGenericMethod(reader.GetType().GetInterface("IAsyncEnumerable`1")!.GetGenericArguments())
.Invoke(this, new object[] { connectionState, kvp.Key.ToString(), reader, cts });
.Invoke(this, [connectionState, kvp.Key.ToString(), reader, cts]);
continue;
}
_ = _sendStreamItemsMethod
.MakeGenericMethod(reader.GetType().GetGenericArguments())
.Invoke(this, new object[] { connectionState, kvp.Key.ToString(), reader, cts });
else
{
if (ReflectionHelper.TryGetStreamType(reader.GetType(), out var channelGenericType))
{
_ = _sendStreamItemsMethod
.MakeGenericMethod(channelGenericType)
.Invoke(this, [connectionState, kvp.Key.ToString(), reader, cts]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is the only place we use the _streamTypeLookup cache, we might as well cache the closed MethodInfo returned by MakGenericMethod or better yet the Func<ConnectionState, string, ChannelReader<T>, CancellationTokenSource, Task> we build from a compiled Expression. Since we're just trying to get the dependency update merged, we can just forget the cache for now.

continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: let's use if ... continue; if ... continue ... or if ... else if ... else. Mixing the continue and if/else blocks is way harder to follow.

}

// Should never get here, we should have already verified the stream types when the user initially calls send/invoke
throw new InvalidOperationException($"{reader.GetType()} is not a {typeof(ChannelReader<>).Name}.");
}
}
}

Expand Down
10 changes: 10 additions & 0 deletions src/SignalR/common/Shared/ReflectionHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Threading.Channels;
Expand All @@ -11,6 +13,8 @@ namespace Microsoft.AspNetCore.SignalR;

internal static class ReflectionHelper
{
private static ConcurrentDictionary<Type, Type> _streamTypeLookup = new();

// mustBeDirectType - Hub methods must use the base 'stream' type and not be a derived class that just implements the 'stream' type
// and 'stream' types from the client are allowed to inherit from accepted 'stream' types
public static bool IsStreamingType([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Interfaces)] Type type, bool mustBeDirectType = false)
Expand All @@ -28,6 +32,9 @@ public static bool IsStreamingType([DynamicallyAccessedMembers(DynamicallyAccess
{
if (nullableType.IsGenericType && nullableType.GetGenericTypeDefinition() == typeof(ChannelReader<>))
{
Debug.Assert(nullableType.GetGenericArguments().Length == 1);

_streamTypeLookup.GetOrAdd(type, nullableType.GetGenericArguments()[0]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we keep the cache, we should use it for repeated calls to IsStreamingType itself. And we should be careful to not use the cache for different values of mustBeDirectType. Maybe we should only cache in the false case.

return true;
}

Expand Down Expand Up @@ -59,4 +66,7 @@ public static bool IsIAsyncEnumerable([DynamicallyAccessedMembers(DynamicallyAcc
}
});
}

public static bool TryGetStreamType(Type stream, [NotNullWhen(true)] out Type? streamType)
=> _streamTypeLookup.TryGetValue(stream, out streamType);
Copy link
Member

@halter73 halter73 Apr 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fragile because it will give a false negative if we don't call IsStreamingType(Type) first. Can we insert null into the cache when the type is not a streaming type, and assert that the key always exists here?

}
Loading