Skip to content

Commit 4887288

Browse files
dustinsoftwareDaniel15
authored andcommitted
Always return JS engine to pool after component render. (#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.
1 parent 51cb42f commit 4887288

File tree

4 files changed

+80
-29
lines changed

4 files changed

+80
-29
lines changed

src/React.AspNet/HtmlHelperExtensions.cs

Lines changed: 49 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -81,17 +81,24 @@ public static IHtmlString React<T>(
8181
string containerClass = null
8282
)
8383
{
84-
var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
85-
if (!string.IsNullOrEmpty(htmlTag))
84+
try
8685
{
87-
reactComponent.ContainerTag = htmlTag;
86+
var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
87+
if (!string.IsNullOrEmpty(htmlTag))
88+
{
89+
reactComponent.ContainerTag = htmlTag;
90+
}
91+
if (!string.IsNullOrEmpty(containerClass))
92+
{
93+
reactComponent.ContainerClass = containerClass;
94+
}
95+
var result = reactComponent.RenderHtml(clientOnly, serverOnly);
96+
return new HtmlString(result);
8897
}
89-
if (!string.IsNullOrEmpty(containerClass))
98+
finally
9099
{
91-
reactComponent.ContainerClass = containerClass;
100+
Environment.ReturnEngineToPool();
92101
}
93-
var result = reactComponent.RenderHtml(clientOnly, serverOnly);
94-
return new HtmlString(result);
95102
}
96103

97104
/// <summary>
@@ -114,31 +121,38 @@ public static IHtmlString ReactWithInit<T>(
114121
T props,
115122
string htmlTag = null,
116123
string containerId = null,
117-
bool clientOnly = false,
124+
bool clientOnly = false,
118125
string containerClass = null
119126
)
120127
{
121-
var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
122-
if (!string.IsNullOrEmpty(htmlTag))
123-
{
124-
reactComponent.ContainerTag = htmlTag;
125-
}
126-
if (!string.IsNullOrEmpty(containerClass))
128+
try
127129
{
128-
reactComponent.ContainerClass = containerClass;
129-
}
130-
var html = reactComponent.RenderHtml(clientOnly);
130+
var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
131+
if (!string.IsNullOrEmpty(htmlTag))
132+
{
133+
reactComponent.ContainerTag = htmlTag;
134+
}
135+
if (!string.IsNullOrEmpty(containerClass))
136+
{
137+
reactComponent.ContainerClass = containerClass;
138+
}
139+
var html = reactComponent.RenderHtml(clientOnly);
131140

132141
#if LEGACYASPNET
133-
var script = new TagBuilder("script")
134-
{
135-
InnerHtml = reactComponent.RenderJavaScript()
136-
};
142+
var script = new TagBuilder("script")
143+
{
144+
InnerHtml = reactComponent.RenderJavaScript()
145+
};
137146
#else
138147
var script = new TagBuilder("script");
139148
script.InnerHtml.AppendHtml(reactComponent.RenderJavaScript());
140149
#endif
141-
return new HtmlString(html + System.Environment.NewLine + script.ToString());
150+
return new HtmlString(html + System.Environment.NewLine + script.ToString());
151+
}
152+
finally
153+
{
154+
Environment.ReturnEngineToPool();
155+
}
142156
}
143157

144158
/// <summary>
@@ -148,18 +162,25 @@ public static IHtmlString ReactWithInit<T>(
148162
/// <returns>JavaScript for all components</returns>
149163
public static IHtmlString ReactInitJavaScript(this IHtmlHelper htmlHelper, bool clientOnly = false)
150164
{
151-
var script = Environment.GetInitJavaScript(clientOnly);
152-
#if LEGACYASPNET
153-
var tag = new TagBuilder("script")
165+
try
154166
{
155-
InnerHtml = script
156-
};
157-
return new HtmlString(tag.ToString());
167+
var script = Environment.GetInitJavaScript(clientOnly);
168+
#if LEGACYASPNET
169+
var tag = new TagBuilder("script")
170+
{
171+
InnerHtml = script
172+
};
173+
return new HtmlString(tag.ToString());
158174
#else
159175
var tag = new TagBuilder("script");
160176
tag.InnerHtml.AppendHtml(script);
161177
return tag;
162178
#endif
179+
}
180+
finally
181+
{
182+
Environment.ReturnEngineToPool();
183+
}
163184
}
164185
}
165186
}

src/React.Core/IReactEnvironment.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,5 +96,10 @@ public interface IReactEnvironment
9696
/// Gets the JSX Transformer for this environment.
9797
/// </summary>
9898
IBabel Babel { get; }
99+
100+
/// <summary>
101+
/// Returns the currently held JS engine to the pool. (no-op if engine pooling is disabled)
102+
/// </summary>
103+
void ReturnEngineToPool();
99104
}
100105
}

src/React.Core/ReactEnvironment.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public class ReactEnvironment : IReactEnvironment, IDisposable
6868
/// Contains an engine acquired from a pool of engines. Only used if
6969
/// <see cref="IReactSiteConfiguration.ReuseJavaScriptEngines"/> is enabled.
7070
/// </summary>
71-
protected readonly Lazy<IJsEngine> _engineFromPool;
71+
protected Lazy<IJsEngine> _engineFromPool;
7272

7373
/// <summary>
7474
/// List of all components instantiated in this environment
@@ -379,9 +379,18 @@ private static string GetVersion()
379379
public virtual void Dispose()
380380
{
381381
_engineFactory.DisposeEngineForCurrentThread();
382+
ReturnEngineToPool();
383+
}
384+
385+
/// <summary>
386+
/// Returns the currently held JS engine to the pool. (no-op if engine pooling is disabled)
387+
/// </summary>
388+
public void ReturnEngineToPool()
389+
{
382390
if (_engineFromPool.IsValueCreated)
383391
{
384392
_engineFactory.ReturnEngineToPool(_engineFromPool.Value);
393+
_engineFromPool = new Lazy<IJsEngine>(() => _engineFactory.GetEngine());
385394
}
386395
}
387396

src/React.Tests/Core/ReactEnvironmentTest.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,21 @@ public void UsesProvidedContainerId()
108108
StringAssert.StartsWith("react_", component2.ContainerId);
109109
}
110110

111+
[Test]
112+
public void ReturnsEngineToPool()
113+
{
114+
var mocks = new Mocks();
115+
var environment = mocks.CreateReactEnvironment();
116+
mocks.Config.Setup(x => x.ReuseJavaScriptEngines).Returns(true);
117+
118+
environment.CreateComponent("ComponentName", new { });
119+
mocks.EngineFactory.Verify(x => x.GetEngine(), Times.Once);
120+
environment.ReturnEngineToPool();
121+
122+
environment.CreateComponent("ComponentName", new { });
123+
mocks.EngineFactory.Verify(x => x.GetEngine(), Times.AtLeast(2));
124+
}
125+
111126
private class Mocks
112127
{
113128
public Mock<IJsEngine> Engine { get; private set; }
@@ -125,6 +140,7 @@ public Mocks()
125140
FileSystem = new Mock<IFileSystem>();
126141
FileCacheHash = new Mock<IFileCacheHash>();
127142

143+
EngineFactory.Setup(x => x.GetEngine()).Returns(Engine.Object);
128144
EngineFactory.Setup(x => x.GetEngineForCurrentThread()).Returns(Engine.Object);
129145
Config.Setup(x => x.LoadBabel).Returns(true);
130146
}

0 commit comments

Comments
 (0)