Skip to content

Commit 3771f24

Browse files
FabianFabian
Fabian
authored and
Fabian
committed
Fixed memory leaks, fixed Excel.Application references in non-global AppDomains
1 parent 501b857 commit 3771f24

File tree

10 files changed

+152
-96
lines changed

10 files changed

+152
-96
lines changed

ExcelScript/AddinFunctions/Evaluate.cs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using ExcelScript.Registration;
33
using ScriptingAbstractions;
44
using System;
5+
using System.Diagnostics;
56
using System.Linq;
67

78
namespace ExcelScript

ExcelScript/AddinFunctions/Parse.cs

-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ public static object Parse(
2929
// CAREFUL: This method may be run in a different AppDomain! Don't reference anything from outside of the delegate
3030
Func<MethodInfo[], MethodInfo> EntryMethodSelector = (methods) =>
3131
{
32-
var _DEBUG_APPD = AppDomain.CurrentDomain.Id;
3332
MethodInfo result = null;
3433

3534
// When only one method is available, we'll just return that one...

ExcelScript/ExcelScriptAddin.cs

+4
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using System.Collections.Generic;
1717
using System.Linq;
1818
using System.Linq.Expressions;
19+
using System.Reflection;
1920
using System.Text;
2021
using Excel = NetOffice.ExcelApi;
2122

@@ -61,6 +62,9 @@ private static IScriptGlobals GlobalsFactory(AppDomain TargetDomain)
6162
if (TargetDomain != AppDomain.CurrentDomain)
6263
throw new InvalidOperationException($"Tried to create script globals for AppDomain {TargetDomain}, but was called in AppDomain {AppDomain.CurrentDomain}");
6364

65+
var excelDnaUtilInitialize = typeof(ExcelDnaUtil).GetMethod("Initialize", BindingFlags.Static | BindingFlags.NonPublic);
66+
excelDnaUtilInitialize.Invoke(null, Array.Empty<object>());
67+
6468
var typename = typeof(Globals).AssemblyQualifiedName;
6569
Type type = Type.GetType(typename);
6670
Func<Excel.Application> applicationFactory = () => new Excel.Application(null, ExcelDna.Integration.ExcelDnaUtil.Application);

Resources/Examples.xlsm

184 Bytes
Binary file not shown.

RoslynScripting/FormatColorScheme.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
namespace RoslynScripting
66
{
7-
[Serializable]
7+
//[Serializable]
88
public struct FormatColorScheme
99
{
1010
public readonly Color Keyword;

RoslynScripting/FormattedText.cs

+12-10
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
namespace RoslynScripting
1010
{
11-
[Serializable]
11+
// [Serializable]
1212
public struct TextSpan
1313
{
1414
public readonly int Start;
@@ -34,8 +34,8 @@ private TextSpan(int start, int length, int end)
3434
}
3535
}
3636

37-
[Serializable]
38-
public class TextFormat
37+
//[Serializable]
38+
public class TextFormat : MarshalByRefObject
3939
{
4040
public readonly Color TextColor;
4141

@@ -45,8 +45,8 @@ public TextFormat(Color TextColor)
4545
}
4646
}
4747

48-
[Serializable]
49-
public class FormattedTextLinePart
48+
// [Serializable]
49+
public class FormattedTextLinePart : MarshalByRefObject
5050
{
5151
/// <summary>
5252
/// Line number of the text
@@ -81,8 +81,8 @@ public override string ToString()
8181
}
8282
}
8383

84-
[Serializable]
85-
public class FormattedTextLine
84+
// [Serializable]
85+
public class FormattedTextLine : MarshalByRefObject
8686
{
8787
/// <summary>
8888
/// The text this line is part of
@@ -121,7 +121,9 @@ public void AddPart(FormattedTextLinePart Part)
121121

122122
public FormattedTextLinePart AppendText(string Text, TextFormat Format)
123123
{
124-
int SpanStart = _Parts.Any() ? _Parts.Last().LineSpan.End : 0;
124+
var lastPart = Parts.Last();
125+
var lastPartLineSpan = lastPart.LineSpan;
126+
int SpanStart = _Parts.Any() ? lastPartLineSpan.End : 0;
125127
int SpanEnd = SpanStart + Text.Length;
126128

127129
var LineSpan = TextSpan.FromBounds(SpanStart, SpanEnd);
@@ -137,8 +139,8 @@ public override string ToString()
137139
}
138140
}
139141

140-
[Serializable]
141-
public class FormattedText
142+
// [Serializable]
143+
public class FormattedText : MarshalByRefObject
142144
{
143145
/// <summary>
144146
/// The lines that make up this text

RoslynScripting/HostedScriptRunner.cs

+91-80
Large diffs are not rendered by default.

RoslynScripting/Parameter.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88

99
namespace RoslynScripting
1010
{
11-
[Serializable]
12-
public class Parameter : IXmlSerializable, IParameter
11+
// [Serializable]
12+
public class Parameter : MarshalByRefObject, IXmlSerializable, IParameter
1313
{
1414
[HashValue]
1515
public string Name { get; set; }

RoslynScripting/Script.cs

+40-1
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,42 @@ internal static async Task<IParseResult<TGlobals>> ParseFromAsync(Func<AppDomain
163163
script.Code = code;
164164
script.m_ScriptingOptions = ScriptingOptions;
165165

166+
// TODO: This is not a great solution.. can we improve this?
167+
// Ideal would be to avoid double-parsing as to not garbage up the appdomain with lot of unneeded assemblies (or having to create individual ones like below)
168+
// but then again we can't use the EntryMethodSelector that nicely anymore
169+
// maybe only compile to semanticmodel and work with that?
170+
var helper = new HostingHelper();
171+
AppDomain domain = null;
172+
ParseResult<TGlobals> result;
173+
174+
try
175+
{
176+
domain = helper.IndividualScriptDomain;
177+
var scriptRunner = (HostedScriptRunner<TGlobals>)Activator.CreateInstance(domain, typeof(HostedScriptRunner<TGlobals>).Assembly.FullName, typeof(HostedScriptRunner<TGlobals>).FullName).Unwrap();
178+
var task = RemoteTask.ClientComplete<RoslynScripting.Internal.IParseResult>(scriptRunner.ParseAsync(script.Code, script.m_ScriptingOptions, EntryMethodSelector, EntryMethodParameterFactory), CancellationToken.None);
179+
180+
var parseResult = await task;
181+
182+
IList<IParameter> marshalledParameters = new List<IParameter>();
183+
184+
foreach (var parameter in parseResult.Parameters)
185+
marshalledParameters.Add(new Parameter { DefaultValue = parameter.DefaultValue, Description = parameter.Description, IsOptional = parameter.IsOptional, Name = parameter.Name, Type = parameter.Type });
186+
187+
script.Code = parseResult.RefactoredCode;
188+
script.Parameters = marshalledParameters;
189+
result = new ParseResult<TGlobals>(script, parseResult.EntryMethodName);
190+
} finally
191+
{
192+
if(domain != null)
193+
AppDomain.Unload(domain);
194+
}
195+
196+
197+
return result;
198+
199+
200+
/*
201+
166202
var hostedScriptRunner = script.GetOrCreateScriptRunner(new IParameter[0], script.Code, script.m_ScriptingOptions);
167203
var task = RemoteTask.ClientComplete<RoslynScripting.Internal.IParseResult>(hostedScriptRunner.ParseAsync(script.Code, script.m_ScriptingOptions, EntryMethodSelector, EntryMethodParameterFactory), CancellationToken.None);
168204
var parseResult = await task;
@@ -171,7 +207,7 @@ internal static async Task<IParseResult<TGlobals>> ParseFromAsync(Func<AppDomain
171207
script.Parameters = parseResult.Parameters.ToList();
172208
173209
var result = new ParseResult<TGlobals>(script, parseResult.EntryMethodName);
174-
return result;
210+
return result;*/
175211
}
176212

177213
public IScriptRunResult Run(IEnumerable<IParameterValue> Parameters)
@@ -230,10 +266,13 @@ private IScriptRunner GetOrCreateScriptRunner(IParameter[] parameters, string sc
230266
// C) the AppDomain in which the script runner is hosted is the same that we would like to use now
231267
if (m_ScriptRunnerCacheInfo != null && !m_ScriptRunnerCacheInfo.ScriptRunner.NeedsRecompilationFor(parameters, scriptCode, Options) && m_ScriptRunnerCacheInfo.HostingDomain == sandbox)
232268
{
269+
Trace.WriteLine("Using cached script runner");
233270
return m_ScriptRunnerCacheInfo.ScriptRunner;
234271
}
235272
else
236273
{
274+
Trace.WriteLine("Creating new script runner");
275+
237276
TryUnregisterLease();
238277

239278
var scriptRunner = (HostedScriptRunner<TGlobals>)Activator.CreateInstance(sandbox, typeof(HostedScriptRunner<TGlobals>).Assembly.FullName, typeof(HostedScriptRunner<TGlobals>).FullName).Unwrap();

ScriptingAbstractions/ScriptingOptions.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public enum HostingType
2727
IndividualScriptAppDomain
2828
}
2929

30-
[Serializable]
30+
// [Serializable]
3131
public class ScriptingOptions : MarshalByRefObject, IXmlSerializable
3232
{
3333
public virtual List<Assembly> References { get; protected set; } = new List<Assembly>();

0 commit comments

Comments
 (0)