Always return JS engine to pool after component render. (reactjs#270)

* Always return JS engine to pool after component render.

This allows other threads to use available engines while other MVC actions are being processed during a page render.

* Add test for ReturnEngineToPool.

Author: dustinsoftware

src/React.AspNet/HtmlHelperExtensions.cs

@@ -81,17 +81,24 @@ public static IHtmlString React<T>(
 			string containerClass = null
 		)
 		{
-			var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
-			if (!string.IsNullOrEmpty(htmlTag))
+			try
 			{
-				reactComponent.ContainerTag = htmlTag;
+				var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
+				if (!string.IsNullOrEmpty(htmlTag))
+				{
+					reactComponent.ContainerTag = htmlTag;
+				}
+				if (!string.IsNullOrEmpty(containerClass))
+				{
+					reactComponent.ContainerClass = containerClass;
+				}
+				var result = reactComponent.RenderHtml(clientOnly, serverOnly);
+				return new HtmlString(result);
 			}
-			if (!string.IsNullOrEmpty(containerClass))
+			finally
 			{
-				reactComponent.ContainerClass = containerClass;
+				Environment.ReturnEngineToPool();
 			}
-			var result = reactComponent.RenderHtml(clientOnly, serverOnly);
-			return new HtmlString(result);
 		}
 
 		/// <summary>
@@ -114,31 +121,38 @@ public static IHtmlString ReactWithInit<T>(
 			T props,
 			string htmlTag = null,
 			string containerId = null,
-            bool clientOnly = false,
+			bool clientOnly = false,
 			string containerClass = null
 		)
 		{
-			var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
-			if (!string.IsNullOrEmpty(htmlTag))
-			{
-				reactComponent.ContainerTag = htmlTag;
-			}
-			if (!string.IsNullOrEmpty(containerClass))
+			try
 			{
-				reactComponent.ContainerClass = containerClass;
-			}
-			var html = reactComponent.RenderHtml(clientOnly);
+				var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
+				if (!string.IsNullOrEmpty(htmlTag))
+				{
+					reactComponent.ContainerTag = htmlTag;
+				}
+				if (!string.IsNullOrEmpty(containerClass))
+				{
+					reactComponent.ContainerClass = containerClass;
+				}
+				var html = reactComponent.RenderHtml(clientOnly);
 
 #if LEGACYASPNET
-			var script = new TagBuilder("script")
-			{
-				InnerHtml = reactComponent.RenderJavaScript()
-			};
+				var script = new TagBuilder("script")
+				{
+					InnerHtml = reactComponent.RenderJavaScript()
+				};
 #else
 			var script = new TagBuilder("script");
 			script.InnerHtml.AppendHtml(reactComponent.RenderJavaScript());
 #endif
-			return new HtmlString(html + System.Environment.NewLine + script.ToString());
+				return new HtmlString(html + System.Environment.NewLine + script.ToString());
+			}
+			finally
+			{
+				Environment.ReturnEngineToPool();
+			}
 		}
 
 		/// <summary>
@@ -148,18 +162,25 @@ public static IHtmlString ReactWithInit<T>(
 		/// <returns>JavaScript for all components</returns>
 		public static IHtmlString ReactInitJavaScript(this IHtmlHelper htmlHelper, bool clientOnly = false)
 		{
-			var script = Environment.GetInitJavaScript(clientOnly);
-#if LEGACYASPNET
-			var tag = new TagBuilder("script")
+			try
 			{
-				InnerHtml = script
-			};
-			return new HtmlString(tag.ToString());
+				var script = Environment.GetInitJavaScript(clientOnly);
+#if LEGACYASPNET
+				var tag = new TagBuilder("script")
+				{
+					InnerHtml = script
+				};
+				return new HtmlString(tag.ToString());
 #else
 			var tag = new TagBuilder("script");
 			tag.InnerHtml.AppendHtml(script);
 			return tag;
 #endif
+			}
+			finally
+			{
+				Environment.ReturnEngineToPool();
+			}
 		}
 	}
 }

src/React.Core/IReactEnvironment.cs

@@ -96,5 +96,10 @@ public interface IReactEnvironment
 		/// Gets the JSX Transformer for this environment.
 		/// </summary>
 		IBabel Babel { get; }
+
+		/// <summary>
+		/// Returns the currently held JS engine to the pool. (no-op if engine pooling is disabled)
+		/// </summary>
+		void ReturnEngineToPool();
 	}
 }

src/React.Core/ReactEnvironment.cs

@@ -68,7 +68,7 @@ public class ReactEnvironment : IReactEnvironment, IDisposable
 		/// Contains an engine acquired from a pool of engines. Only used if 
 		/// <see cref="IReactSiteConfiguration.ReuseJavaScriptEngines"/> is enabled.
 		/// </summary>
-		protected readonly Lazy<IJsEngine> _engineFromPool;
+		protected Lazy<IJsEngine> _engineFromPool;
 
 		/// <summary>
 		/// List of all components instantiated in this environment
@@ -379,9 +379,18 @@ private static string GetVersion()
 		public virtual void Dispose()
 		{
 			_engineFactory.DisposeEngineForCurrentThread();
+			ReturnEngineToPool();
+		}
+
+		/// <summary>
+		/// Returns the currently held JS engine to the pool. (no-op if engine pooling is disabled)
+		/// </summary>
+		public void ReturnEngineToPool()
+		{
 			if (_engineFromPool.IsValueCreated)
 			{
 				_engineFactory.ReturnEngineToPool(_engineFromPool.Value);
+				_engineFromPool = new Lazy<IJsEngine>(() => _engineFactory.GetEngine());
 			}
 		}
 

src/React.Tests/Core/ReactEnvironmentTest.cs

@@ -108,6 +108,21 @@ public void UsesProvidedContainerId()
 			StringAssert.StartsWith("react_", component2.ContainerId);
 		}
 
+		[Test]
+		public void ReturnsEngineToPool()
+		{
+			var mocks = new Mocks();
+			var environment = mocks.CreateReactEnvironment();
+			mocks.Config.Setup(x => x.ReuseJavaScriptEngines).Returns(true);
+
+			environment.CreateComponent("ComponentName", new { });
+			mocks.EngineFactory.Verify(x => x.GetEngine(), Times.Once);
+			environment.ReturnEngineToPool();
+
+			environment.CreateComponent("ComponentName", new { });
+			mocks.EngineFactory.Verify(x => x.GetEngine(), Times.AtLeast(2));
+		}
+
 		private class Mocks
 		{
 			public Mock<IJsEngine> Engine { get; private set; }
@@ -125,6 +140,7 @@ public Mocks()
 				FileSystem = new Mock<IFileSystem>();
 				FileCacheHash = new Mock<IFileCacheHash>();
 
+				EngineFactory.Setup(x => x.GetEngine()).Returns(Engine.Object);
 				EngineFactory.Setup(x => x.GetEngineForCurrentThread()).Returns(Engine.Object);
 				Config.Setup(x => x.LoadBabel).Returns(true);
 			}