Disposal improvements for ScriptHost

Author: mathewc

src/WebJobs.Script.WebHost/Diagnostics/FunctionInstanceLogger.cs

@@ -79,8 +79,7 @@ internal FunctionInstanceLogger(
                         // This exception will cause the function to not get executed.
                         throw new InvalidOperationException($"Missing function.json for '{shortName}'.");
                     }
-                    Description.FunctionLogger logInfo = descr.Invoker.LogInfo;
-                    state = new FunctionInstanceMonitor(descr.Metadata, _metrics, item.FunctionInstanceId, logInfo);
+                    state = new FunctionInstanceMonitor(descr.Metadata, _metrics, item.FunctionInstanceId, descr.Invoker.FunctionLogger);
 
                     item.Properties[Key] = state;
 

src/WebJobs.Script/Binding/FunctionTraceBinderProvider.cs

@@ -25,8 +25,6 @@ public static void Create(ExtensionConfigContext context)
 
             Func<string, FunctionDescriptor> funcLookup = context.Config.GetService<Func<string, FunctionDescriptor>>();
 
-            // Func<string, FunctionDescriptor> funcLookup = null;
-
             if (funcLookup != null && inner != null)
             {
                 IBindingProvider wrapper = new Wrapper(inner, funcLookup);
@@ -86,10 +84,10 @@ private async Task<IValueProvider> WrapAsync(IValueProvider result, ValueBinding
                     {
                         var shortName = context.FunctionContext.MethodName;
                         FunctionDescriptor descr = _parent._funcLookup(shortName);
-                        var logInfo = descr.Invoker.LogInfo;
+                        var functionLogger = descr.Invoker.FunctionLogger;
 
                         // This is the critical call
-                        trace = logInfo.CreateUserTraceWriter(trace);
+                        trace = functionLogger.CreateUserTraceWriter(trace);
 
                         return new SimpleValueProvider(typeof(TraceWriter), trace, result.ToInvokeString());
                     }

src/WebJobs.Script/Description/FunctionInvokerBase.cs

@@ -23,16 +23,11 @@ public abstract class FunctionInvokerBase : IFunctionInvoker, IDisposable
         private bool _disposed = false;
         private IDisposable _fileChangeSubscription;
 
-        internal FunctionInvokerBase(ScriptHost host, FunctionMetadata functionMetadata)
-            : this(host, functionMetadata, new FunctionLogger(host, functionMetadata.Name))
-        {
-        }
-
-        internal FunctionInvokerBase(ScriptHost host, FunctionMetadata functionMetadata, FunctionLogger logInfo)
+        internal FunctionInvokerBase(ScriptHost host, FunctionMetadata functionMetadata, string logDirName = null)
         {
             Host = host;
             Metadata = functionMetadata;
-            LogInfo = logInfo;
+            FunctionLogger = new FunctionLogger(host, logDirName ?? functionMetadata.Name);
         }
 
         protected static IDictionary<string, object> PrimaryHostTraceProperties { get; }
@@ -46,15 +41,15 @@ internal FunctionInvokerBase(ScriptHost host, FunctionMetadata functionMetadata,
 
         public ScriptHost Host { get; }
 
-        public FunctionLogger LogInfo { get; }
+        public FunctionLogger FunctionLogger { get; }
 
         public FunctionMetadata Metadata { get; }
 
-        protected TraceWriter TraceWriter => LogInfo.TraceWriter;
+        protected TraceWriter TraceWriter => FunctionLogger.TraceWriter;
 
-        protected ILogger Logger => LogInfo.Logger;
+        protected ILogger Logger => FunctionLogger.Logger;
 
-        public TraceWriter FileTraceWriter => LogInfo.FileTraceWriter;
+        public TraceWriter FileTraceWriter => FunctionLogger.FileTraceWriter;
 
         /// <summary>
         /// All unhandled invocation exceptions will flow through this method.
@@ -70,7 +65,7 @@ public virtual void OnError(Exception ex)
 
         protected virtual void TraceError(string errorMessage)
         {
-            LogInfo.TraceError(errorMessage);
+            FunctionLogger.TraceError(errorMessage);
         }
 
         protected bool InitializeFileWatcherIfEnabled()
@@ -155,7 +150,7 @@ internal void TraceCompilationDiagnostics(ImmutableArray<Diagnostic> diagnostics
                 return;
             }
 
-            TraceWriter traceWriter = LogInfo.TraceWriter;
+            TraceWriter traceWriter = FunctionLogger.TraceWriter;
             IDictionary<string, object> properties = PrimaryHostTraceProperties;
 
             if (!logTarget.HasFlag(LogTargets.User))
@@ -208,7 +203,7 @@ protected virtual void Dispose(bool disposing)
                 {
                     _fileChangeSubscription?.Dispose();
 
-                    (LogInfo.TraceWriter as IDisposable)?.Dispose();
+                    FunctionLogger.Dispose();
                 }
 
                 _disposed = true;

src/WebJobs.Script/Description/FunctionLogger.cs

@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
+using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using Microsoft.Azure.WebJobs.Host;
@@ -11,8 +12,10 @@ namespace Microsoft.Azure.WebJobs.Script.Description
 {
     // Each function can get its own log stream.
     // Static per-function logging information.
-    public class FunctionLogger
+    public class FunctionLogger : IDisposable
     {
+        private bool _disposed = false;
+
         public FunctionLogger(ScriptHost host, string functionName, string logDirName = null)
         {
             // Function file logging is only done conditionally
@@ -82,5 +85,25 @@ public void LogFunctionResult(bool success, string invocationId, long elapsedMs)
             TraceWriter.Trace(message, traceWriterLevel, null);
             Logger?.Log(logLevel, new EventId(0), message, null, (s, e) => s);
         }
+
+        protected virtual void Dispose(bool disposing)
+        {
+            if (!_disposed)
+            {
+                if (disposing)
+                {
+                    (FileTraceWriter as IDisposable)?.Dispose();
+                    (TraceWriter as IDisposable)?.Dispose();
+                }
+
+                _disposed = true;
+            }
+        }
+
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
     }
 }

src/WebJobs.Script/Description/IFunctionInvoker.cs

@@ -11,7 +11,7 @@ public interface IFunctionInvoker
         /// <summary>
         /// Gets logging information for this function.
         /// </summary>
-        FunctionLogger LogInfo { get; }
+        FunctionLogger FunctionLogger { get; }
 
         /// <summary>
         /// Invoke the function using the specified parameters.

src/WebJobs.Script/Description/Proxies/ProxyFunctionInvoker.cs

@@ -2,12 +2,8 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
 using System.Linq;
-using System.Net.Http;
-using System.Text;
 using System.Threading.Tasks;
-using Microsoft.Azure.AppService.Proxy.Client.Contract;
 using Microsoft.AspNetCore.Http;
 
 namespace Microsoft.Azure.WebJobs.Script.Description
@@ -17,7 +13,7 @@ internal class ProxyFunctionInvoker : FunctionInvokerBase
         private ProxyClientExecutor _proxyClient;
 
         public ProxyFunctionInvoker(ScriptHost host, FunctionMetadata functionMetadata, ProxyClientExecutor proxyClient)
-            : base(host, functionMetadata, new FunctionLogger(host, functionMetadata.Name, logDirName: "Proxy"))
+            : base(host, functionMetadata, logDirName: "Proxy")
         {
             _proxyClient = proxyClient;
         }

src/WebJobs.Script/Host/PrimaryHostCoordinator.cs

@@ -203,7 +203,10 @@ private void ResetLease()
 
         private void SetTimerInterval(TimeSpan interval, TimeSpan? dueTimeout = null)
         {
-            _timer.Change(dueTimeout ?? interval, interval);
+            if (!_disposed)
+            {
+                _timer.Change(dueTimeout ?? interval, interval);
+            }
         }
 
         private void TryReleaseLeaseIfOwned()