Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Make ResourceManager use Assembly instead of RuntimeAssembly internally. #21979

Merged
jkotas merged 6 commits into from
Jan 27, 2019
Merged

Make ResourceManager use Assembly instead of RuntimeAssembly internally. #21979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
12f9f0f
Make ResourceManager use Assembly instead of RuntimeAssembly internally.
filipnavara Jan 14, 2019
8d2b63e
Fix exception name.
filipnavara Jan 14, 2019
a5a1fc2
Fix method qualifiers.
filipnavara Jan 14, 2019
4f6f30a
Add generic implementation of GetNeutralResourcesLanguageAttribute.
filipnavara Jan 15, 2019
a343b92
PR feedback (removed SimpleName optimization, added back restriction …
filipnavara Jan 25, 2019
dba3e80
Move function accidentally added to shared partition.
filipnavara Jan 26, 2019
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
4 changes: 4 additions & 0 deletions src/System.Private.CoreLib/src/System/Reflection/Assembly.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,5 +253,9 @@ public static Assembly GetEntryAssembly()
GetEntryAssembly(JitHelpers.GetObjectHandleOnStack(ref entryAssembly));
return entryAssembly;
}

// Exists to faciliate code sharing between CoreCLR and CoreRT.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal bool IsRuntimeImplemented() => this is RuntimeAssembly;
}
}
21 changes: 12 additions & 9 deletions src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ internal static RuntimeAssembly InternalLoadAssemblyName(
bool throwOnFileNotFound,
IntPtr ptrLoadContextBinder = default)
{
return InternalLoadAssemblyName(assemblyRef, reqAssembly, ref stackMark, IntPtr.Zero, true /*throwOnError*/, ptrLoadContextBinder);
return InternalLoadAssemblyName(assemblyRef, reqAssembly, ref stackMark, IntPtr.Zero, throwOnFileNotFound, ptrLoadContextBinder);
Copy link
Copy Markdown
Member Author

@filipnavara filipnavara Jan 14, 2019

Choose a reason for hiding this comment

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

This is not directly related to the PR, but it should be fixed nevertheless. The method takes the throwOnFileNotFound parameter, but it didn't actually use it.

}

internal static RuntimeAssembly InternalLoadAssemblyName(
Expand Down Expand Up @@ -646,15 +646,13 @@ public override Assembly GetSatelliteAssembly(CultureInfo culture, Version versi
if (culture == null)
throw new ArgumentNullException(nameof(culture));

string name = GetSimpleName() + ".resources";
return InternalGetSatelliteAssembly(name, culture, version, true);
return InternalGetSatelliteAssembly(culture, version, true);
Copy link
Copy Markdown
Member Author

@filipnavara filipnavara Jan 14, 2019

Choose a reason for hiding this comment

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

I moved the name generation to InternalGetSatelliteAssembly to avoid unnecessarily duplicating it in ManifestBasedResourceGroveler.

}

[System.Security.DynamicSecurityMethod] // Methods containing StackCrawlMark local var has to be marked DynamicSecurityMethod
internal RuntimeAssembly InternalGetSatelliteAssembly(string name,
CultureInfo culture,
Version version,
bool throwOnFileNotFound)
internal Assembly InternalGetSatelliteAssembly(CultureInfo culture,
Version version,
bool throwOnFileNotFound)
{
// This stack crawl mark is never used because the requesting assembly is explicitly specified,
// so the value could be anything.
Expand All @@ -670,13 +668,18 @@ internal RuntimeAssembly InternalGetSatelliteAssembly(string name,
an.Version = version;

an.CultureInfo = culture;
an.Name = name;
an.Name = GetSimpleName() + ".resources";

RuntimeAssembly retAssembly = nLoad(an, null, this, ref stackMark,
IntPtr.Zero,
throwOnFileNotFound);

if (retAssembly == this || (retAssembly == null && throwOnFileNotFound))
if (retAssembly == this)
{
retAssembly = null;
}

if (retAssembly == null && throwOnFileNotFound)
{
throw new FileNotFoundException(string.Format(culture, SR.IO_FileNotFound_FileName, an.Name));
}
Expand Down
57 changes: 30 additions & 27 deletions src/System.Private.CoreLib/src/System/Resources/ManifestBasedResourceGroveler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public ResourceSet GrovelForResourceSet(CultureInfo culture, Dictionary<string,

ResourceSet rs = null;
Stream stream = null;
RuntimeAssembly satellite = null;
Assembly satellite = null;

// 1. Fixups for ultimate fallbacks
CultureInfo lookForCulture = UltimateFallbackFixup(culture);
Expand Down Expand Up @@ -145,9 +145,7 @@ internal static CultureInfo GetNeutralResourcesLanguage(Assembly a, ref Ultimate
Debug.Assert(a != null, "assembly != null");
string cultureName = null;
short fallback = 0;
if (GetNeutralResourcesLanguageAttribute(((RuntimeAssembly)a).GetNativeHandle(),
JitHelpers.GetStringHandleOnStack(ref cultureName),
out fallback))
if (GetNeutralResourcesLanguageAttribute(a, ref cultureName, out fallback))
Comment thread
filipnavara marked this conversation as resolved.
{
if ((UltimateResourceFallbackLocation)fallback < UltimateResourceFallbackLocation.MainAssembly || (UltimateResourceFallbackLocation)fallback > UltimateResourceFallbackLocation.Satellite)
{
Expand Down Expand Up @@ -228,7 +226,7 @@ internal ResourceSet CreateResourceSet(Stream store, Assembly assembly)
// resMgrHeaderVersion is older than this ResMgr version.
// We should add in backwards compatibility support here.

throw new NotSupportedException(SR.Format(SR.NotSupported_ObsoleteResourcesFile, _mediator.MainAssembly.GetSimpleName()));
throw new NotSupportedException(SR.Format(SR.NotSupported_ObsoleteResourcesFile, _mediator.MainAssembly.GetName().Name));
}

store.Position = startPos;
Expand Down Expand Up @@ -306,7 +304,7 @@ internal ResourceSet CreateResourceSet(Stream store, Assembly assembly)
}
}

private Stream GetManifestResourceStream(RuntimeAssembly satellite, string fileName)
private Stream GetManifestResourceStream(Assembly satellite, string fileName)
{
Debug.Assert(satellite != null, "satellite shouldn't be null; check caller");
Debug.Assert(fileName != null, "fileName shouldn't be null; check caller");
Expand All @@ -323,8 +321,8 @@ private Stream GetManifestResourceStream(RuntimeAssembly satellite, string fileN
// Looks up a .resources file in the assembly manifest using
// case-insensitive lookup rules. Yes, this is slow. The metadata
// dev lead refuses to make all assembly manifest resource lookups case-insensitive,
// even optionally case-insensitive.
private Stream CaseInsensitiveManifestResourceStreamLookup(RuntimeAssembly satellite, string name)
// even optionally case-insensitive.
private Stream CaseInsensitiveManifestResourceStreamLookup(Assembly satellite, string name)
{
Debug.Assert(satellite != null, "satellite shouldn't be null; check caller");
Debug.Assert(name != null, "name shouldn't be null; check caller");
Expand Down Expand Up @@ -360,24 +358,23 @@ private Stream CaseInsensitiveManifestResourceStreamLookup(RuntimeAssembly satel
return satellite.GetManifestResourceStream(canonicalName);
}

private RuntimeAssembly GetSatelliteAssembly(CultureInfo lookForCulture)
private Assembly GetSatelliteAssembly(CultureInfo lookForCulture)
{
if (!_mediator.LookedForSatelliteContractVersion)
{
_mediator.SatelliteContractVersion = _mediator.ObtainSatelliteContractVersion(_mediator.MainAssembly);
_mediator.LookedForSatelliteContractVersion = true;
}

RuntimeAssembly satellite = null;
string satAssemblyName = GetSatelliteAssemblyName();
Assembly satellite = null;

// Look up the satellite assembly, but don't let problems
// like a partially signed satellite assembly stop us from
// doing fallback and displaying something to the user.
// Yet also somehow log this error for a developer.
try
{
satellite = _mediator.MainAssembly.InternalGetSatelliteAssembly(satAssemblyName, lookForCulture, _mediator.SatelliteContractVersion, false);
satellite = InternalGetSatelliteAssembly(_mediator.MainAssembly, lookForCulture, _mediator.SatelliteContractVersion);
}

// Jun 08: for cases other than ACCESS_DENIED, we'll assert instead of throw to give release builds more opportunity to fallback.
Expand All @@ -390,14 +387,14 @@ private RuntimeAssembly GetSatelliteAssembly(CultureInfo lookForCulture)
int hr = fle._HResult;
if (hr != Win32Marshal.MakeHRFromErrorCode(Interop.Errors.ERROR_ACCESS_DENIED))
{
Debug.Fail("[This assert catches satellite assembly build/deployment problems - report this message to your build lab & loc engineer]" + Environment.NewLine + "GetSatelliteAssembly failed for culture " + lookForCulture.Name + " and version " + (_mediator.SatelliteContractVersion == null ? _mediator.MainAssembly.GetVersion().ToString() : _mediator.SatelliteContractVersion.ToString()) + " of assembly " + _mediator.MainAssembly.GetSimpleName() + " with error code 0x" + hr.ToString("X", CultureInfo.InvariantCulture) + Environment.NewLine + "Exception: " + fle);
Debug.Fail("[This assert catches satellite assembly build/deployment problems - report this message to your build lab & loc engineer]" + Environment.NewLine + "GetSatelliteAssembly failed for culture " + lookForCulture.Name + " and version " + (_mediator.SatelliteContractVersion == null ? _mediator.MainAssembly.GetName().Version.ToString() : _mediator.SatelliteContractVersion.ToString()) + " of assembly " + _mediator.MainAssembly.GetName().Name + " with error code 0x" + hr.ToString("X", CultureInfo.InvariantCulture) + Environment.NewLine + "Exception: " + fle);
}
}

// Don't throw for zero-length satellite assemblies, for compat with v1
catch (BadImageFormatException bife)
{
Debug.Fail("[This assert catches satellite assembly build/deployment problems - report this message to your build lab & loc engineer]" + Environment.NewLine + "GetSatelliteAssembly failed for culture " + lookForCulture.Name + " and version " + (_mediator.SatelliteContractVersion == null ? _mediator.MainAssembly.GetVersion().ToString() : _mediator.SatelliteContractVersion.ToString()) + " of assembly " + _mediator.MainAssembly.GetSimpleName() + Environment.NewLine + "Exception: " + bife);
Debug.Fail("[This assert catches satellite assembly build/deployment problems - report this message to your build lab & loc engineer]" + Environment.NewLine + "GetSatelliteAssembly failed for culture " + lookForCulture.Name + " and version " + (_mediator.SatelliteContractVersion == null ? _mediator.MainAssembly.GetName().Version.ToString() : _mediator.SatelliteContractVersion.ToString()) + " of assembly " + _mediator.MainAssembly.GetName().Name + Environment.NewLine + "Exception: " + bife);
}

return satellite;
Expand Down Expand Up @@ -436,24 +433,15 @@ private bool CanUseDefaultResourceClasses(string readerTypeName, string resSetTy
return true;
}

private string GetSatelliteAssemblyName()
{
string satAssemblyName = _mediator.MainAssembly.GetSimpleName();
satAssemblyName += ".resources";
return satAssemblyName;
}

private void HandleSatelliteMissing()
{
string satAssemName = _mediator.MainAssembly.GetSimpleName() + ".resources.dll";
string satAssemName = _mediator.MainAssembly.GetName().Name + ".resources.dll";
if (_mediator.SatelliteContractVersion != null)
{
satAssemName += ", Version=" + _mediator.SatelliteContractVersion.ToString();
}

AssemblyName an = new AssemblyName();
an.SetPublicKey(_mediator.MainAssembly.GetPublicKey());
byte[] token = an.GetPublicKeyToken();
byte[] token = _mediator.MainAssembly.GetName().GetPublicKeyToken();

int iLen = token.Length;
StringBuilder publicKeyTok = new StringBuilder(iLen * 2);
Expand Down Expand Up @@ -489,11 +477,26 @@ private void HandleResourceStreamMissing(string fileName)
if (_mediator.LocationInfo != null && _mediator.LocationInfo.Namespace != null)
resName = _mediator.LocationInfo.Namespace + Type.Delimiter;
resName += fileName;
throw new MissingManifestResourceException(SR.Format(SR.MissingManifestResource_NoNeutralAsm, resName, _mediator.MainAssembly.GetSimpleName()));
throw new MissingManifestResourceException(SR.Format(SR.MissingManifestResource_NoNeutralAsm, resName, _mediator.MainAssembly.GetName().Name));
}

// Internal version of GetSatelliteAssembly that avoids throwing FileNotFoundException
private static Assembly InternalGetSatelliteAssembly(Assembly mainAssembly,
CultureInfo culture,
Version version)
{
Comment thread
filipnavara marked this conversation as resolved.
return ((RuntimeAssembly)mainAssembly).InternalGetSatelliteAssembly(culture, version, throwOnFileNotFound: false);
}

[DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
[return: MarshalAs(UnmanagedType.Bool)]
internal static extern bool GetNeutralResourcesLanguageAttribute(RuntimeAssembly assemblyHandle, StringHandleOnStack cultureName, out short fallbackLocation);
private static extern bool GetNeutralResourcesLanguageAttribute(RuntimeAssembly assemblyHandle, StringHandleOnStack cultureName, out short fallbackLocation);

private static bool GetNeutralResourcesLanguageAttribute(Assembly assemblyHandle, ref string cultureName, out short fallbackLocation)
{
return GetNeutralResourcesLanguageAttribute(((RuntimeAssembly)assemblyHandle).GetNativeHandle(),
JitHelpers.GetStringHandleOnStack(ref cultureName),
out fallbackLocation);
}
}
}
28 changes: 11 additions & 17 deletions src/System.Private.CoreLib/src/System/Resources/ResourceManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,9 @@ public ResourceManager(string baseName, Assembly assembly)
{
if (null == baseName)
throw new ArgumentNullException(nameof(baseName));

if (null == assembly)
throw new ArgumentNullException(nameof(assembly));

if (!(assembly is RuntimeAssembly))
Comment thread
filipnavara marked this conversation as resolved.
if (!assembly.IsRuntimeImplemented())
throw new ArgumentException(SR.Argument_MustBeRuntimeAssembly);

MainAssembly = assembly;
Expand All @@ -242,8 +240,7 @@ public ResourceManager(string baseName, Assembly assembly, Type usingResourceSet
throw new ArgumentNullException(nameof(baseName));
if (null == assembly)
throw new ArgumentNullException(nameof(assembly));

if (!(assembly is RuntimeAssembly))
if (!assembly.IsRuntimeImplemented())
throw new ArgumentException(SR.Argument_MustBeRuntimeAssembly);

MainAssembly = assembly;
Expand All @@ -260,8 +257,7 @@ public ResourceManager(Type resourceSource)
{
if (null == resourceSource)
throw new ArgumentNullException(nameof(resourceSource));

if (!(resourceSource is RuntimeType))
if (!resourceSource.IsRuntimeImplemented())
throw new ArgumentException(SR.Argument_MustBeRuntimeType);

_locationInfo = resourceSource;
Expand Down Expand Up @@ -664,7 +660,7 @@ internal static WindowsRuntimeResourceManagerBase GetWinRTResourceManager()
//
// b) For any other non-FX assembly, we will use the modern resource manager with the premise that app package
// contains the PRI resources.
private bool ShouldUseSatelliteAssemblyResourceLookupUnderAppX(RuntimeAssembly resourcesAssembly)
private bool ShouldUseSatelliteAssemblyResourceLookupUnderAppX(Assembly resourcesAssembly)
{
bool fUseSatelliteAssemblyResourceLookupUnderAppX = typeof(object).Assembly == resourcesAssembly;

Expand Down Expand Up @@ -706,11 +702,9 @@ private void SetAppXConfiguration()

bool bUsingSatelliteAssembliesUnderAppX = false;

RuntimeAssembly resourcesAssembly = (RuntimeAssembly)MainAssembly;

if (resourcesAssembly != null)
if (MainAssembly != null)
{
if (resourcesAssembly != typeof(object).Assembly) // We are not loading resources for mscorlib
if (MainAssembly != typeof(object).Assembly) // We are not loading resources for mscorlib
{
if (ApplicationModel.IsUap)
{
Expand All @@ -731,7 +725,7 @@ private void SetAppXConfiguration()

if (!bUsingSatelliteAssembliesUnderAppX)
{
_bUsingModernResourceManagement = !ShouldUseSatelliteAssemblyResourceLookupUnderAppX(resourcesAssembly);
_bUsingModernResourceManagement = !ShouldUseSatelliteAssemblyResourceLookupUnderAppX(MainAssembly);

if (_bUsingModernResourceManagement)
{
Expand All @@ -749,7 +743,7 @@ private void SetAppXConfiguration()

try
{
_PRIonAppXInitialized = _WinRTResourceManager.Initialize(resourcesAssembly.Location, reswFilename, out _PRIExceptionInfo);
_PRIonAppXInitialized = _WinRTResourceManager.Initialize(MainAssembly.Location, reswFilename, out _PRIExceptionInfo);
// Note that _PRIExceptionInfo might be null - this is OK.
// In that case we will just throw the generic
// MissingManifestResource_NoPRIresources exception.
Expand Down Expand Up @@ -801,7 +795,7 @@ private void SetAppXConfiguration()
}
}
}
// resourcesAssembly == null should not happen but it can. See the comment on Assembly.GetCallingAssembly.
// MainAssembly == null should not happen but it can. See the comment on Assembly.GetCallingAssembly.
// However for the sake of 100% backwards compatibility on Win7 and below, we must leave
// _bUsingModernResourceManagement as false.
#endif // FEATURE_APPX
Expand Down Expand Up @@ -1069,9 +1063,9 @@ internal UltimateResourceFallbackLocation FallbackLoc
set { _rm._fallbackLoc = value; }
}

internal RuntimeAssembly MainAssembly
internal Assembly MainAssembly
{
get { return (RuntimeAssembly)_rm.MainAssembly; }
get { return _rm.MainAssembly; }
}

// this is weird because we have BaseNameField accessor above, but we're sticking
Expand Down