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

Fixing the OOM in scripting #75372

Merged
merged 13 commits into from
Oct 10, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -3206,10 +3206,12 @@ public void Metadata_References_Provider()
class C { }
";
var parseOptions = TestOptions.RegularPreview;
var metadataRefs = new[] {
PortableExecutableReference[] metadataRefs =
[
MetadataReference.CreateFromAssemblyInternal(this.GetType().Assembly),
MetadataReference.CreateFromAssemblyInternal(typeof(object).Assembly)
};
];

Compilation compilation = CreateEmptyCompilation(source, options: TestOptions.DebugDllThrowing, parseOptions: parseOptions, references: metadataRefs);
compilation.VerifyDiagnostics();
Assert.Single(compilation.SyntaxTrees);
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.IO;
using System.Linq;
using System.Reflection;
using System.Reflection.PortableExecutable;
using System.Security.Cryptography;
using System.Text;
using System.Threading;
Expand Down Expand Up @@ -210,7 +211,7 @@ internal virtual Func<string, MetadataReferenceProperties, PortableExecutableRef
return (path, properties) =>
{
var peStream = FileSystem.OpenFileWithNormalizedException(path, FileMode.Open, FileAccess.Read, FileShare.Read);
return MetadataReference.CreateFromFile(peStream, path, properties);
return MetadataReference.CreateFromFile(peStream, path, PEStreamOptions.PrefetchEntireImage, properties, documentation: null);
jaredpar marked this conversation as resolved.
Show resolved Hide resolved
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,21 +241,35 @@ public static PortableExecutableReference CreateFromStream(
public static PortableExecutableReference CreateFromFile(
string path,
MetadataReferenceProperties properties = default,
DocumentationProvider? documentation = null)
=> CreateFromFile(
DocumentationProvider? documentation = null) =>
CreateFromFile(
StandardFileSystem.Instance.OpenFileWithNormalizedException(path, FileMode.Open, FileAccess.Read, FileShare.Read),
path,
PEStreamOptions.PrefetchEntireImage,
properties,
documentation);

internal static MetadataImageReference CreateFromFile(
string path,
PEStreamOptions options,
MetadataReferenceProperties properties,
DocumentationProvider? documentation = null) =>
CreateFromFile(
StandardFileSystem.Instance.OpenFileWithNormalizedException(path, FileMode.Open, FileAccess.Read, FileShare.Read),
path,
options,
properties,
documentation);

internal static PortableExecutableReference CreateFromFile(
internal static MetadataImageReference CreateFromFile(
Stream peStream,
string path,
MetadataReferenceProperties properties = default,
PEStreamOptions options,
MetadataReferenceProperties properties,
DocumentationProvider? documentation = null)
{
// prefetch image, close stream to avoid locking it:
var module = ModuleMetadata.CreateFromStream(peStream, PEStreamOptions.PrefetchEntireImage);
var module = ModuleMetadata.CreateFromStream(peStream, options);

if (properties.Kind == MetadataImageKind.Module)
{
Expand Down Expand Up @@ -287,7 +301,7 @@ public static MetadataReference CreateFromAssembly(Assembly assembly)
return CreateFromAssemblyInternal(assembly);
}

internal static MetadataReference CreateFromAssemblyInternal(Assembly assembly)
internal static MetadataImageReference CreateFromAssemblyInternal(Assembly assembly)
{
return CreateFromAssemblyInternal(assembly, default(MetadataReferenceProperties));
}
Expand Down Expand Up @@ -318,14 +332,10 @@ public static MetadataReference CreateFromAssembly(
return CreateFromAssemblyInternal(assembly, properties, documentation);
}

internal static PortableExecutableReference CreateFromAssemblyInternal(
internal static string GetAssemblyFilePath(
Assembly assembly,
MetadataReferenceProperties properties,
DocumentationProvider? documentation = null)
MetadataReferenceProperties properties)
{
// Note: returns MetadataReference and not PortableExecutableReference so that we can in future support assemblies that
333fred marked this conversation as resolved.
Show resolved Hide resolved
// which are not backed by PE image.

if (assembly == null)
{
throw new ArgumentNullException(nameof(assembly));
Expand All @@ -347,13 +357,20 @@ internal static PortableExecutableReference CreateFromAssemblyInternal(
throw new NotSupportedException(CodeAnalysisResources.CantCreateReferenceToAssemblyWithoutLocation);
}

Stream peStream = StandardFileSystem.Instance.OpenFileWithNormalizedException(location, FileMode.Open, FileAccess.Read, FileShare.Read);
return location;
}

internal static MetadataImageReference CreateFromAssemblyInternal(
Assembly assembly,
MetadataReferenceProperties properties,
DocumentationProvider? documentation = null)
{
var filePath = GetAssemblyFilePath(assembly, properties);
var peStream = StandardFileSystem.Instance.OpenFileWithNormalizedException(filePath, FileMode.Open, FileAccess.Read, FileShare.Read);

// The file is locked by the CLR assembly loader, so we can create a lazily read metadata,
// which might also lock the file until the reference is GC'd.
var metadata = AssemblyMetadata.CreateFromStream(peStream);

return new MetadataImageReference(metadata, properties, documentation, location, display: null);
return CreateFromFile(peStream, filePath, PEStreamOptions.Default, properties, documentation);
}

internal static bool HasMetadata(Assembly assembly)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator.ExpressionCompiler.UnitTests" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator.ResultProvider.UnitTests" />
<InternalsVisibleTo Include="InteractiveHost.UnitTests" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.Scripting.TestUtilities" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.Scripting.UnitTests" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.Scripting.Desktop.UnitTests" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.CSharp.Scripting.UnitTests" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ private static RuntimeMetadataReferenceResolver CreateMetadataReferenceResolver(
baseDirectory,
gacFileResolver: platformInfo.HasGlobalAssemblyCache ? new GacFileResolver(preferredCulture: CultureInfo.CurrentCulture) : null,
platformAssemblyPaths: platformInfo.PlatformAssemblyPaths,
fileReferenceProvider: (path, properties) => metadataService.GetReference(path, properties));
createFromFileFunc: metadataService.GetReference);
}

private static SourceReferenceResolver CreateSourceReferenceResolver(ImmutableArray<string> searchPaths, string baseDirectory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ private static CompilationOptions GetCompilationOptionsWithScriptReferenceResolv
var referenceResolver = RuntimeMetadataReferenceResolver.CreateCurrentPlatformResolver(
searchPaths: [RuntimeEnvironment.GetRuntimeDirectory()],
baseDirectory: baseDirectory,
fileReferenceProvider: metadataService.GetReference);
createFromFileFunc: metadataService.GetReference);

return compilationOptions
.WithMetadataReferenceResolver(referenceResolver)
Expand Down
21 changes: 13 additions & 8 deletions src/Scripting/CSharp/Hosting/CommandLine/Csi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System;
using System.IO;
using System.Reflection;
using System.Reflection.PortableExecutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Scripting;
Expand All @@ -16,18 +15,24 @@ namespace Microsoft.CodeAnalysis.CSharp.Scripting.Hosting
{
internal sealed class CSharpInteractiveCompiler : CSharpCompiler
{
internal CSharpInteractiveCompiler(string responseFile, BuildPaths buildPaths, string[] args, IAnalyzerAssemblyLoader analyzerLoader)
private readonly Func<string, PEStreamOptions, MetadataReferenceProperties, MetadataImageReference> _createFromFileFunc;

internal CSharpInteractiveCompiler(
string? responseFile,
BuildPaths buildPaths,
string[] args,
IAnalyzerAssemblyLoader analyzerLoader,
Func<string, PEStreamOptions, MetadataReferenceProperties, MetadataImageReference>? createFromFileFunc = null)
// Unlike C# compiler we do not use LIB environment variable. It's only supported for historical reasons.
: base(CSharpCommandLineParser.Script, responseFile, args, buildPaths, null, analyzerLoader)
: base(CSharpCommandLineParser.Script, responseFile, args, buildPaths, additionalReferenceDirectories: null, analyzerLoader)
{
_createFromFileFunc = createFromFileFunc ?? Script.CreateFromFile;
}

internal override Type Type => typeof(CSharpInteractiveCompiler);

internal override MetadataReferenceResolver GetCommandLineMetadataReferenceResolver(TouchedFileLogger loggerOpt)
{
return CommandLineRunner.GetMetadataReferenceResolver(Arguments, loggerOpt);
}
internal override MetadataReferenceResolver GetCommandLineMetadataReferenceResolver(TouchedFileLogger? loggerOpt) =>
CommandLineRunner.GetMetadataReferenceResolver(Arguments, loggerOpt, _createFromFileFunc);

public override void PrintLogo(TextWriter consoleOutput)
{
Expand Down
Loading