When a file is changed, wee currently wait for all outstanding functions to exit and then pick up the new changes.

Change this to orphan the existing ScriptHost (stop listening, but finish current functions), and immediately spin up a new host.

Author: MikeStall

src/WebJobs.Script/Host/ScriptHostManager.cs

@@ -2,7 +2,10 @@
 // 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.Threading;
+using System.Threading.Tasks;
 
 namespace Microsoft.Azure.WebJobs.Script
 {
@@ -13,8 +16,16 @@ namespace Microsoft.Azure.WebJobs.Script
     public class ScriptHostManager : IDisposable
     {
         private readonly ScriptHostConfiguration _config;
-        private ScriptHost _instance;
+        private ScriptHost _currentInstance;
+
+        // ScriptHosts are not thread safe, so be clear that only 1 thread at a time operates on each instance. 
+        // List of all outstanding ScriptHost instances. Only 1 of these (specified by _currentInstance)
+        // should be listening at a time. The others are "orphaned" and exist to finish executing any functions 
+        // and will then remove themselves from this list. 
+        private HashSet<ScriptHost> _liveInstances = new HashSet<ScriptHost>();
+
         private bool _stopped;
+        private AutoResetEvent _stopEvent = new AutoResetEvent(false);
 
         public ScriptHostManager(ScriptHostConfiguration config)
         {
@@ -31,7 +42,7 @@ public ScriptHost Instance
         {
             get
             {
-                return _instance;
+                return _currentInstance;
             }
         }
 
@@ -47,7 +58,11 @@ public ScriptHost Instance
                 ScriptHost newInstance = ScriptHost.Create(_config);
 
                 newInstance.Start();
-                _instance = newInstance;
+                lock (_liveInstances)
+                {
+                    _liveInstances.Add(newInstance);
+                }
+                _currentInstance = newInstance;
                 OnHostStarted();
 
                 // only after ALL initialization is complete do we set this flag
@@ -58,31 +73,69 @@ public ScriptHost Instance
                 // 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.
-                _instance.RestartEvent.WaitOne();
+                WaitHandle.WaitAny(new WaitHandle[] {
+                    newInstance.RestartEvent,
+                    _stopEvent
+                });
 
-                // stop the host fully
-                _instance.Stop();
-                _instance.Dispose();
+                // 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);
         }
 
+        // Let the existing host instance finsih currently executing functions.
+        private async Task Orphan(ScriptHost instance)
+        {
+            lock (_liveInstances)
+            {
+                bool removed = _liveInstances.Remove(instance);
+                if (!removed)
+                {
+                    return; // somebody else is handling it
+                }
+            }
+
+            // this thread now owns the instance
+            await instance.StopAsync();
+            instance.Dispose();
+        }
+
         public void Stop()
         {
             _stopped = true;
 
             try
             {
-                if (_instance != null)
+                _stopEvent.Set();
+                ScriptHost[] instances = GetLiveInstancesAndClear();
+
+                Task[] tasksStop = Array.ConvertAll(instances, instance => instance.StopAsync());
+                Task.WaitAll(tasksStop);
+
+                foreach (var instance in instances)
                 {
-                    _instance.Stop();
-                    _instance.Dispose();
+                    instance.Dispose();
                 }
             }
             catch
             {
                 // best effort
-            }  
+            }
+        }
+
+        private ScriptHost[] GetLiveInstancesAndClear()
+        {
+            ScriptHost[] instances;
+            lock (_liveInstances)
+            {
+                instances = _liveInstances.ToArray();
+                _liveInstances.Clear();
+            }
+
+            return instances;
         }
 
         protected virtual void OnHostStarted()
@@ -91,9 +144,10 @@ protected virtual void OnHostStarted()
 
         public void Dispose()
         {
-            if (_instance != null)
+            ScriptHost[] instances = GetLiveInstancesAndClear();
+            foreach (var instance in instances)
             {
-                _instance.Dispose();
+                instance.Dispose();
             }
         }
     }

test/WebJobs.Script.Tests/NodeEndToEndTests.cs

@@ -10,6 +10,7 @@
 using System.Threading.Tasks;
 using Newtonsoft.Json.Linq;
 using Xunit;
+using System.Diagnostics;
 
 namespace WebJobs.Script.Tests
 {
@@ -143,12 +144,32 @@ public async Task WebHookTrigger_GenericJson()
         [Fact]
         public async Task TimerTrigger()
         {
-            // job is running every second, so give it a few seconds to
-            // generate some output
-            await Task.Delay(4000);
+            var stopwatch = Stopwatch.StartNew();
+
+            while (true)
+            {
+                // Time will write to this file. 
+                try
+                {
+                    string[] lines = File.ReadAllLines("joblog.txt");
+                    if (lines.Length > 2)
+                    {
+                        return;
+                    }
+                }
+                catch
+                {
+                    // File may be missing if timer hasn't written yet. 
+                }
+
+                if (stopwatch.ElapsedMilliseconds > 6*1000)
+                {
+                    Assert.True(false, "Timeout waiting for timer to fire.");
+                }
+
+                await Task.Delay(TimeSpan.FromSeconds(1));
+            }
 
-            string[] lines = File.ReadAllLines("joblog.txt");
-            Assert.True(lines.Length > 2);
         }
 
         public class TestFixture : EndToEndTestFixture

test/WebJobs.Script.Tests/ScriptHostManagerTests.cs

@@ -0,0 +1,98 @@
+// 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.IO;
+using System.Net;
+using System.Net.Http;
+using System.Net.Http.Headers;
+using System.Threading.Tasks;
+using Newtonsoft.Json.Linq;
+using Xunit;
+using System.Diagnostics;
+using Microsoft.Azure.WebJobs.Script;
+using System.Threading;
+using Microsoft.WindowsAzure.Storage.Blob;
+
+namespace WebJobs.Script.Tests
+{
+    [Trait("Category", "E2E")]
+    public class ScriptHostManagerTests 
+    {
+        // Update a script file (the function.json) to force the ScriptHost to re-index and pick up new changes. 
+        // Test with timers: 
+        [Fact]
+        public async Task UpdateFileAndRestart()
+        {
+            Random r = new Random();
+
+            CancellationTokenSource cts = new CancellationTokenSource();
+
+            var fixture = new NodeEndToEndTests.TestFixture();
+            var blob1 = UpdateOutputName("outputblobname", "first", fixture);
+
+            await fixture.Host.StopAsync();
+            var config = fixture.Host.ScriptConfig;            
+
+            using (var manager = new ScriptHostManager(config))
+            {
+                // Background task to run while the main thread is pumping events at RunAndBlock(). 
+                Thread t = new Thread(_ =>
+                   {
+                       // Wait for initial execution.
+                       Wait(blob1);
+
+                       // This changes the bindings so that we now write to blob2
+                       var blob2 = UpdateOutputName("first", "second", fixture);
+
+                       // wait for newly executed 
+                       Wait(blob2);
+
+                       manager.Stop();
+                   });
+                t.Start();                           
+
+                manager.RunAndBlock(cts.Token);
+
+                t.Join();
+            }
+        }
+
+        // For a blob to appear. This is evidence that the function ran. 
+        void Wait(CloudBlockBlob blob)
+        {
+            Stopwatch sw = Stopwatch.StartNew();
+            const int timeoutMs = 10 * 1000; // 
+
+            while (!blob.Exists())
+            {
+                Thread.Sleep(500);
+                if (sw.ElapsedMilliseconds > timeoutMs)
+                {
+                    // If no blob appeared yet, then the function didn't run. 
+                    // It may have not picked up the new changes
+                    Assert.True(false, "Timeout waiting for blob to appear. " + timeoutMs + "ms");
+                }
+            }
+        }
+
+        // Update the manifest for the timer function
+        // - this will cause a file touch which cause ScriptHostManager to notice and update
+        // - set to a new output location so that we can ensure we're getting new changes. 
+        static CloudBlockBlob UpdateOutputName(string prev, string hint, EndToEndTestFixture fixture)
+        {
+            string name = hint;
+
+            string manifestPath = Path.Combine(Environment.CurrentDirectory, @"TestScripts\Node\TimerTrigger\function.json");
+            string content = File.ReadAllText(manifestPath);            
+            content = content.Replace(prev, name);
+            File.WriteAllText(manifestPath, content);
+
+            var blob = fixture.TestContainer.GetBlockBlobReference(name);
+            blob.DeleteIfExists();
+            return blob;
+
+        }
+    }
+}

test/WebJobs.Script.Tests/TestScripts/Node/TimerTrigger/function.json

@@ -5,6 +5,13 @@
         "type": "timerTrigger",
         "schedule": "* * * * * *"
       }
+    ],
+    "output": [
+      {
+        "type": "blob",
+        "name": "output",
+        "path": "test-output/outputblobname"
+      }
     ]
   }
 }

test/WebJobs.Script.Tests/TestScripts/Node/TimerTrigger/index.js

@@ -1,8 +1,9 @@
 var fs = require('fs');
 
 module.exports = function (context) {
-    var timeStamp = new Date().toISOString();
+    var timeStamp = new Date().toISOString();    
     fs.appendFile('joblog.txt', timeStamp + '\r\n', function (err) {
-        context.done(err);
+        var blobOutputContents = "from timer trigger: " + timeStamp;
+        context.done(err, blobOutputContents);
     });
 }

test/WebJobs.Script.Tests/WebJobs.Script.Tests.csproj

@@ -178,6 +178,7 @@
     <Compile Include="PhpEndToEndTests.cs" />
     <Compile Include="PowershellEndToEndTests.cs" />
     <Compile Include="PythonEndToEndTests.cs" />
+    <Compile Include="ScriptHostManagerTests.cs" />
     <Compile Include="ScriptHostTests.cs" />
     <Compile Include="NodeEndToEndTests.cs" />
     <Compile Include="NodeFunctionGenerationTests.cs" />