Skip to content

Commit

Permalink
Fix first round of trim warnings (#2451)
Browse files Browse the repository at this point in the history
- DefaultOptionsProvider.TryGetAzureRoleInstanceIdNoThrow use Type.GetType to check for Microsoft.WindowsAzure.ServiceRuntime.RoleEnvironment, so the trimmer knows to preserve this type, if it is in the app
- ChannelMessage use the added public API for ChannelReader CanCount and Count, but fallback to reflection on netcoreapp3.1, since those APIs are not available on that runtime.

Contributes to #2449

This at least removes the warnings from using `Microsoft.Extensions.Caching.StackExchangeRedis`.

We could also add a trimming test app as outlined in https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming#show-all-warnings-with-sample-application, along with a unit test that publishes the app and ensures there are no new warnings. LMK if you think this is valuable.

There are still these warnings left in this library:

```
/_/src/StackExchange.Redis/ScriptParameterMapper.cs(173): Trim analysis warning IL2070: StackExchange.Redis.ScriptParameterMapper.IsValidParameterHash(Type,LuaScript,String&,String&): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.PublicNestedTypes', 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.PublicEvents' in call to 'System.Type.GetMember(String)'. The parameter 't' of method 'StackExchange.Redis.ScriptParameterMapper.IsValidParameterHash(Type,LuaScript,String&,String&)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/StackExchange.Redis/ScriptParameterMapper.cs(227): Trim analysis warning IL2070: StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.PublicNestedTypes', 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.PublicEvents' in call to 'System.Type.GetMember(String)'. The parameter 't' of method 'StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/StackExchange.Redis/ScriptParameterMapper.cs(260): Trim analysis warning IL2026: StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript): Using member 'System.Linq.Expressions.Expression.Property(Expression,String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/StackExchange.Redis/ScriptParameterMapper.cs(261): Trim analysis warning IL2026: StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript): Using member 'System.Linq.Expressions.Expression.Call(Expression,String,Type[],Expression[])' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
```

Fixing those will require a bit more work, as the whole LuaScript functionality looks like it needs to be marked `RequiresUnreferencedCode`.

cc @mgravell @NickCraver
  • Loading branch information
eerhardt authored May 1, 2023
1 parent 7ad0add commit 3ba8d2a
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 17 deletions.
1 change: 1 addition & 0 deletions docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Current package versions:

- Fix [#2426](https://github.com/StackExchange/StackExchange.Redis/issues/2426): Don't restrict multi-slot operations on Envoy proxy; let the proxy decide ([#2428 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2428))
- Add: Support for `User`/`Password` in `DefaultOptionsProvider` to support token rotation scenarios ([#2445 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2445))
- Fix [#2449](https://github.com/StackExchange/StackExchange.Redis/issues/2449): Resolve AOT trim warnings in `TryGetAzureRoleInstanceIdNoThrow` ([#2451 by eerhardt](https://github.com/StackExchange/StackExchange.Redis/pull/2451))

## 2.6.104

Expand Down
11 changes: 10 additions & 1 deletion src/StackExchange.Redis/ChannelMessageQueue.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Channels;
using System.Threading.Tasks;
Expand Down Expand Up @@ -126,6 +125,7 @@ public ValueTask<ChannelMessage> ReadAsync(CancellationToken cancellationToken =
/// <param name="count">The (approximate) count of items in the Channel.</param>
public bool TryGetCount(out int count)
{
#if NETCOREAPP3_1
// get this using the reflection
try
{
Expand All @@ -137,6 +137,15 @@ public bool TryGetCount(out int count)
}
}
catch { }
#else
var reader = _queue.Reader;
if (reader.CanCount)
{
count = reader.Count;
return true;
}
#endif

count = default;
return false;
}
Expand Down
22 changes: 6 additions & 16 deletions src/StackExchange.Redis/Configuration/DefaultOptionsProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -225,31 +225,21 @@ protected virtual string GetDefaultClientName() =>
string? roleInstanceId;
try
{
Assembly? asm = null;
foreach (var asmb in AppDomain.CurrentDomain.GetAssemblies())
{
if (asmb.GetName()?.Name?.Equals("Microsoft.WindowsAzure.ServiceRuntime") == true)
{
asm = asmb;
break;
}
}
if (asm == null)
return null;

var type = asm.GetType("Microsoft.WindowsAzure.ServiceRuntime.RoleEnvironment");
var roleEnvironmentType = Type.GetType("Microsoft.WindowsAzure.ServiceRuntime.RoleEnvironment, Microsoft.WindowsAzure.ServiceRuntime", throwOnError: false);

// https://msdn.microsoft.com/en-us/library/microsoft.windowsazure.serviceruntime.roleenvironment.isavailable.aspx
if (type?.GetProperty("IsAvailable") is not PropertyInfo isAvailableProp
if (roleEnvironmentType?.GetProperty("IsAvailable") is not PropertyInfo isAvailableProp
|| isAvailableProp.GetValue(null, null) is not bool isAvailableVal
|| !isAvailableVal)
{
return null;
}

var currentRoleInstanceProp = type.GetProperty("CurrentRoleInstance");
var currentRoleInstanceProp = roleEnvironmentType.GetProperty("CurrentRoleInstance");
var currentRoleInstanceId = currentRoleInstanceProp?.GetValue(null, null);
roleInstanceId = currentRoleInstanceId?.GetType().GetProperty("Id")?.GetValue(currentRoleInstanceId, null)?.ToString();

var roleInstanceType = Type.GetType("Microsoft.WindowsAzure.ServiceRuntime.RoleInstance, Microsoft.WindowsAzure.ServiceRuntime", throwOnError: false);
roleInstanceId = roleInstanceType?.GetProperty("Id")?.GetValue(currentRoleInstanceId, null)?.ToString();

if (roleInstanceId.IsNullOrEmpty())
{
Expand Down

0 comments on commit 3ba8d2a

Please sign in to comment.