Simplifying node initialization, avoiding locks and removing sync paths

Author: fabiocav

src/WebJobs.Script/Description/DotNet/DotNetFunctionInvoker.cs

@@ -35,7 +35,7 @@ public sealed class DotNetFunctionInvoker : FunctionInvokerBase
 
         private FunctionSignature _functionSignature;
         private IFunctionMetadataResolver _metadataResolver;
-        private Action _reloadScript;
+        private Func<Task> _reloadScript;
         private Action _shutdown;
         private Action _restorePackages;
         private Action<MethodInfo, object[], object[], object> _resultProcessor;
@@ -63,7 +63,7 @@ internal DotNetFunctionInvoker(ScriptHost host, FunctionMetadata functionMetadat
 
             _functionLoader = new FunctionLoader<MethodInfo>(CreateFunctionTarget);
 
-            _reloadScript = ReloadScript;
+            _reloadScript = ReloadScriptAsync;
             _reloadScript = _reloadScript.Debounce();
 
             _shutdown = () => Host.Shutdown();
@@ -120,7 +120,7 @@ public override void OnError(Exception ex)
             TraceError(error);
         }
 
-        private void ReloadScript()
+        private async Task ReloadScriptAsync()
         {
             // Reset cached function
             _functionLoader.Reset();
@@ -156,7 +156,7 @@ private void ReloadScript()
                 (_functionSignature == null ||
                 (_functionSignature.HasLocalTypeReference || !_functionSignature.Equals(signature))))
             {
-                Host.Restart();
+                await Host.RestartAsync().ConfigureAwait(false);
             }
         }
 
@@ -205,7 +205,7 @@ internal async Task RestorePackagesAsync(bool reloadScriptOnSuccess = true)
                     }
                     else
                     {
-                        _reloadScript();
+                        await _reloadScript();
                     }
                 }
             }

src/WebJobs.Script/Description/Node/NodeFunctionInvoker.cs

@@ -2,7 +2,6 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Diagnostics;
@@ -12,6 +11,7 @@
 using System.Linq;
 using System.Net.Http;
 using System.Net.Http.Headers;
+using System.Threading;
 using System.Threading.Tasks;
 using EdgeJs;
 using Microsoft.Azure.WebJobs.Host;
@@ -30,15 +30,14 @@ public class NodeFunctionInvoker : FunctionInvokerBase
         private readonly string _script;
         private readonly BindingMetadata _trigger;
         private readonly string _entryPoint;
-        private static readonly object _initializationSyncRoot = new object();
 
         private Func<object, Task<object>> _scriptFunc;
         private static Func<object, Task<object>> _clearRequireCache;
         private static Func<object, Task<object>> _globalInitializationFunc;
         private static string _functionTemplate;
         private static string _clearRequireCacheScript;
         private static string _globalInitializationScript;
-        private static bool _initialized = false;
+        private static Lazy<Task> _initializer = new Lazy<Task>(InitializeAsync, LazyThreadSafetyMode.ExecutionAndPublication);
 
         static NodeFunctionInvoker()
         {
@@ -109,7 +108,8 @@ private static Func<object, Task<object>> ClearRequireCacheFunc
 
         protected override async Task InvokeCore(object[] parameters, FunctionInvocationContext context)
         {
-            EnsureInitialized();
+            // Ensure we're properly initialized
+            await _initializer.Value.ConfigureAwait(false);
 
             object input = parameters[0];
             string invocationId = context.ExecutionContext.InvocationId.ToString();
@@ -126,21 +126,6 @@ protected override async Task InvokeCore(object[] parameters, FunctionInvocation
             await ProcessOutputBindingsAsync(_outputBindings, input, context.Binder, bindingData, scriptExecutionContext, functionResult);
         }
 
-        private static void EnsureInitialized()
-        {
-            if (!_initialized)
-            {
-                lock (_initializationSyncRoot)
-                {
-                    if (!_initialized)
-                    {
-                        Initialize();
-                        _initialized = true;
-                    }
-                }
-            }
-        }
-
         private async Task ProcessInputBindingsAsync(Binder binder, Dictionary<string, object> executionContext, Dictionary<string, object> bindingData)
         {
             var bindings = (Dictionary<string, object>)executionContext["bindings"];
@@ -238,9 +223,9 @@ private static async Task ProcessOutputBindingsAsync(Collection<FunctionBinding>
             }
         }
 
-        internal static void OnHostRestart()
+        internal static async Task OnHostRestartAsync()
         {
-            ClearRequireCacheFunc(null).GetAwaiter().GetResult();
+            await ClearRequireCacheFunc(null);
         }
 
         protected override void OnScriptFileChanged(object sender, FileSystemEventArgs e)
@@ -621,7 +606,7 @@ private static bool TryDeserializeJson<TResult>(string json, out TResult result)
         /// <summary>
         /// Performs required static initialization in the Edge context.
         /// </summary>
-        private static void Initialize()
+        private static async Task InitializeAsync()
         {
             var handle = (Func<object, Task<object>>)(err =>
             {
@@ -642,7 +627,7 @@ private static void Initialize()
                 { "handleUncaughtException", handle }
             };
 
-            GlobalInitializationFunc(context).GetAwaiter().GetResult();
+            await GlobalInitializationFunc(context);
         }
     }
 }

src/WebJobs.Script/Extensions/FuncExtensions.cs

@@ -0,0 +1,37 @@
+// 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.Threading;
+using System.Threading.Tasks;
+
+namespace Microsoft.Azure.WebJobs.Script
+{
+    public static class FuncExtensions
+    {
+        public static Func<T, Task> Debounce<T>(this Func<T, Task> func, int milliseconds = 300)
+        {
+            var last = 0;
+
+            return async (arg) =>
+            {
+                var current = Interlocked.Increment(ref last);
+
+                await Task.Delay(milliseconds);
+
+                if (current == last)
+                {
+                    await func(arg);
+                }
+            };
+        }
+
+        public static Func<Task> Debounce(this Func<Task> targetAction, int milliseconds = 300)
+        {
+            Func<object, Task> action = _ => targetAction();
+            action = action.Debounce(milliseconds);
+
+            return () => action(null);
+        }
+    }
+}

src/WebJobs.Script/Host/ScriptHost.cs

@@ -42,7 +42,7 @@ public class ScriptHost : JobHost
         internal const string GeneratedTypeName = "Functions";
         private readonly IScriptHostEnvironment _scriptHostEnvironment;
         private string _instanceId;
-        private Action _restart;
+        private Func<Task> _restart;
         private Action _shutdown;
         private AutoRecoveringFileSystemWatcher _scriptFileWatcher;
         private AutoRecoveringFileSystemWatcher _debugModeFileWatcher;
@@ -349,7 +349,7 @@ protected virtual void Initialize()
                 // This allows us to deal with a large set of file change events that might
                 // result from a bulk copy/unzip operation. In such cases, we only want to
                 // restart after ALL the operations are complete and there is a quiet period.
-                _restart = Restart;
+                _restart = RestartAsync;
                 _restart = _restart.Debounce(500);
 
                 _shutdown = Shutdown;
@@ -444,7 +444,7 @@ internal static Collection<CustomAttributeBuilder> CreateTypeAttributes(ScriptHo
             return customAttributes;
         }
 
-        internal void Restart()
+        internal async Task RestartAsync()
         {
             if (_shutdownScheduled)
             {
@@ -458,7 +458,7 @@ internal void Restart()
 #if FEATURE_NODE
             // whenever we're restarting the host, we want to let the Node
             // invoker know so it can clear the require cache, etc.
-            NodeFunctionInvoker.OnHostRestart();
+            await NodeFunctionInvoker.OnHostRestartAsync();
 #endif
         }
 
@@ -1233,11 +1233,12 @@ private void OnFileChanged(object sender, FileSystemEventArgs e)
                 }
 
                 TraceFileChangeRestart(e.ChangeType.ToString(), e.FullPath, shutdown);
-                ScheduleRestart(shutdown);
+                ScheduleRestartAsync(shutdown).ContinueWith(t => TraceWriter.Error($"Error restarting host (full shutdown: {shutdown})", t.Exception),
+                    TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.OnlyOnFaulted);
             }
         }
 
-        private void ScheduleRestart(bool shutdown)
+        private async Task ScheduleRestartAsync(bool shutdown)
         {
             if (shutdown)
             {
@@ -1246,7 +1247,7 @@ private void ScheduleRestart(bool shutdown)
             }
             else
             {
-                _restart();
+                await _restart();
             }
         }
 

src/WebJobs.Script/WebJobs.Script.csproj

@@ -423,6 +423,7 @@
     <Compile Include="Extensions\AssemblyExtensions.cs" />
     <Compile Include="Extensions\ExceptionExtensions.cs" />
     <Compile Include="Extensions\FileUtility.cs" />
+    <Compile Include="Extensions\FuncExtensions.cs" />
     <Compile Include="Extensions\HttpRequestMessageExtensions.cs" />
     <Compile Include="Extensions\IDictionaryExtensions.cs" />
     <Compile Include="Extensions\IMetricsLoggerExtensions.cs" />