Making ScriptHostManager error handling more robust (Keeping host alive)

Author: mathewc

CustomDictionary.xml

@@ -73,6 +73,7 @@
       <Word>ScopeId</Word>
       <Word>FunctionMetadata</Word>
       <Word>Debounce</Word>
+      <Word>ScriptHost</Word>
 	</Recognized>
     <Deprecated/>
     <Compound>
@@ -92,6 +93,7 @@
 		<Term>SingletonAttribute</Term>
 		<Term>ScopeId</Term>
     <Term>FunctionMetadata</Term>
+    <Term>ScriptHost</Term>
     </DiscreteExceptions>
   </Words>
   <Acronyms>

src/WebJobs.Script.WebHost/App_Start/AutofacBootstrap.cs

@@ -60,7 +60,7 @@ internal static void Initialize(ContainerBuilder builder)
             WebHookReceiverManager webHookRecieverManager = new WebHookReceiverManager(secretManager);
             builder.RegisterInstance<WebHookReceiverManager>(webHookRecieverManager);
 
-            Task.Run(() => scriptHostManager.RunAndBlock(CancellationToken.None));
+            HostingEnvironment.QueueBackgroundWorkItem((ct) => scriptHostManager.RunAndBlock(ct));
         }
     }
 }

src/WebJobs.Script/Host/ScriptHost.cs

@@ -24,7 +24,6 @@ public class ScriptHost : JobHost
         private const string HostAssemblyName = "ScriptHost";
         private const string HostConfigFileName = "host.json";
         internal const string FunctionConfigFileName = "function.json";
-        private readonly TraceWriter _traceWriter;
         private readonly AutoResetEvent _restartEvent = new AutoResetEvent(false);
         private Action<FileSystemEventArgs> _restart;
         private FileSystemWatcher _fileWatcher;
@@ -38,12 +37,12 @@ protected ScriptHost(ScriptHostConfiguration scriptConfig)
             if (scriptConfig.FileLoggingEnabled)
             {
                 string hostLogFilePath = Path.Combine(scriptConfig.RootLogPath, "Host");
-                _traceWriter = new FileTraceWriter(hostLogFilePath, TraceLevel.Verbose);
-                scriptConfig.HostConfig.Tracing.Tracers.Add(_traceWriter);
+                TraceWriter = new FileTraceWriter(hostLogFilePath, TraceLevel.Verbose);
+                scriptConfig.HostConfig.Tracing.Tracers.Add(TraceWriter);
             }
             else
             {
-                _traceWriter = NullTraceWriter.Instance;
+                TraceWriter = NullTraceWriter.Instance;
             }
 
             if (scriptConfig.TraceWriter != null)
@@ -75,8 +74,8 @@ protected ScriptHost(ScriptHostConfiguration scriptConfig)
             // restart after ALL the operations are complete and there is a quiet period.
             _restart = (e) =>
             {
-                _traceWriter.Verbose(string.Format(CultureInfo.InvariantCulture, "File change of type '{0}' detected for '{1}'", e.ChangeType, e.FullPath));
-                _traceWriter.Verbose("Host configuration has changed. Signaling restart.");
+                TraceWriter.Verbose(string.Format(CultureInfo.InvariantCulture, "File change of type '{0}' detected for '{1}'", e.ChangeType, e.FullPath));
+                TraceWriter.Verbose("Host configuration has changed. Signaling restart.");
 
                 // signal host restart
                 _restartEvent.Set();
@@ -87,6 +86,8 @@ protected ScriptHost(ScriptHostConfiguration scriptConfig)
             _directoryCountSnapshot = Directory.EnumerateDirectories(ScriptConfig.RootScriptPath).Count();
         }
 
+        public TraceWriter TraceWriter { get; private set; }
+
         public ScriptHostConfiguration ScriptConfig { get; private set; }
 
         public Collection<FunctionDescriptor> Functions { get; private set; }
@@ -134,7 +135,7 @@ protected virtual void Initialize()
                 File.WriteAllText(hostConfigFilePath, "{}");
             }
 
-            _traceWriter.Verbose(string.Format(CultureInfo.InvariantCulture, "Reading host configuration file '{0}'", hostConfigFilePath));
+            TraceWriter.Verbose(string.Format(CultureInfo.InvariantCulture, "Reading host configuration file '{0}'", hostConfigFilePath));
             string json = File.ReadAllText(hostConfigFilePath);
             JObject hostConfig = JObject.Parse(json);
             ApplyConfiguration(hostConfig, ScriptConfig);
@@ -143,7 +144,7 @@ protected virtual void Initialize()
             Collection<FunctionDescriptor> functions = ReadFunctions(ScriptConfig, descriptionProviders);
             string defaultNamespace = "Host";
             string typeName = string.Format(CultureInfo.InvariantCulture, "{0}.{1}", defaultNamespace, "Functions");
-            _traceWriter.Verbose(string.Format(CultureInfo.InvariantCulture, "Generating {0} job function(s)", functions.Count));
+            TraceWriter.Verbose(string.Format(CultureInfo.InvariantCulture, "Generating {0} job function(s)", functions.Count));
             Type type = FunctionGenerator.Generate(HostAssemblyName, typeName, functions);
             List<Type> types = new List<Type>();
             types.Add(type);
@@ -168,14 +169,13 @@ public static ScriptHost Create(ScriptHostConfiguration scriptConfig = null)
             }
 
             ScriptHost scriptHost = new ScriptHost(scriptConfig);
-
             try
             {
                 scriptHost.Initialize();
             }
-            catch (Exception e)
+            catch (Exception ex)
             {
-                scriptHost._traceWriter.Error("Script Host initialization failed", e);
+                scriptHost.TraceWriter.Error("ScriptHost initialization failed", ex);
                 throw;
             }
 

src/WebJobs.Script/Host/ScriptHostManager.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.Azure.WebJobs.Host;
 
 namespace Microsoft.Azure.WebJobs.Script
 {
@@ -27,6 +28,7 @@ public class ScriptHostManager : IDisposable
         private bool _disposed;
         private bool _stopped;
         private AutoResetEvent _stopEvent = new AutoResetEvent(false);
+        private TraceWriter _traceWriter;
 
         public ScriptHostManager(ScriptHostConfiguration config)
         {
@@ -53,44 +55,60 @@ public ScriptHost Instance
             // host level configuration files change
             do
             {
-                IsRunning = false;
-
-                // Create a new host config, but keep the host id from existing one
-                _config.HostConfig = new JobHostConfiguration
+                try
                 {
-                    HostId = _config.HostConfig.HostId
-                };
-                ScriptHost newInstance = ScriptHost.Create(_config);
-
-                // TODO: consider using StartAsync here to speed up
-                // restarts.
-                newInstance.Start();
-                lock (_liveInstances)
+                    IsRunning = false;
+
+                    // Create a new host config, but keep the host id from existing one
+                    _config.HostConfig = new JobHostConfiguration
+                    {
+                        HostId = _config.HostConfig.HostId
+                    };
+                    ScriptHost newInstance = ScriptHost.Create(_config);
+                    _traceWriter = newInstance.TraceWriter;
+
+                    newInstance.Start();
+                    lock (_liveInstances)
+                    {
+                        _liveInstances.Add(newInstance);
+                    }
+                    _currentInstance = newInstance;
+                    OnHostStarted();
+
+                    // only after ALL initialization is complete do we set this flag
+                    IsRunning = true;
+
+                    // Wait for a restart signal. This event will automatically reset.
+                    // While we're restarting, it is possible for another restart to be
+                    // signaled. That is fine - the restart will be processed immediately
+                    // once we get to this line again. The important thing is that these
+                    // restarts are only happening on a single thread.
+                    WaitHandle.WaitAny(new WaitHandle[] {
+                        newInstance.RestartEvent,
+                        _stopEvent
+                    });
+
+                    // Orphan the current host instance. We're stopping it, so it won't listen for any new functions
+                    // it will finish any currently executing functions and then clean itself up.
+                    // Spin around and create a new host instance.
+                    Task tIgnore = Orphan(newInstance);
+                }
+                catch (Exception ex)
                 {
-                    _liveInstances.Add(newInstance);
+                    // We need to keep the host running, so we catch and log any errors
+                    // then restart the host
+                    if (_traceWriter != null)
+                    {
+                        _traceWriter.Error("A ScriptHost error occurred", ex);
+                    }
+
+                    // Wait for a short period of time before restarting to
+                    // avoid cases where a host level config error might cause
+                    // a rapid restart cycle
+                    Task.Delay(5000).Wait();
                 }
-                _currentInstance = newInstance;
-                OnHostStarted();
-
-                // only after ALL initialization is complete do we set this flag
-                IsRunning = true;
-
-                // Wait for a restart signal. This event will automatically reset.
-                // While we're restarting, it is possible for another restart to be
-                // signaled. That is fine - the restart will be processed immediately
-                // once we get to this line again. The important thing is that these
-                // restarts are only happening on a single thread.
-                WaitHandle.WaitAny(new WaitHandle[] {
-                    newInstance.RestartEvent,
-                    _stopEvent
-                });
-
-                // Orphan the current host instance. We're stopping it, so it won't listen for any new functions
-                // it will finish any currently executing functions and then clean itself up.
-                // Spin around and create a new host instance.
-                Task tIgnore = Orphan(newInstance);
             }
-            while (!_stopped);
+            while (!_stopped && !cancellationToken.IsCancellationRequested);
         }
 
         // Let the existing host instance finish currently executing functions.