InvalidOperationException during JavaScriptEngineFactory.GetFactory

LinusCenterstrom commented 5 years ago

Thanks for filing a bug! To save time, if you're having trouble using the library, please check off the items you have tried. If you are just asking a question, skip right to the bottom.

Please verify these steps before filing an issue, and check them off as you go

[x] The relevant native JavascriptEngineSwitcher library packages are installed (such as JavaScriptEngineSwitcher.V8.Native.win-x64)
[x] The VC++ 2017 runtime is installed
[x] The value of SetUseReact and SetUseBabel is correct in ReactConfig.cs or Startup.cs
[x] I've looked at the sample projects in this repo to verify that my app is configured correctly

I'm using these library versions:

ReactJS.NET: 3.2.0
JavaScriptEngineSwitcher: 2.4.9
react and react-dom: (N/A if using bundled react, or version number) 16.8
webpack: (N/A if using bundled react) 4
node: (N/A if using bundled react) 10

Runtime environment:

OS: (Mac, Windows, Linux flavor. Include 32-bit/64-bit and version) Windows
.NET Framework or .NET Core Version: 4.7.1 and 4.7.2 (different projects)

Steps to reproduce

Unclear, but here is what happens. Every now and then the javascript engine factory throws during initialization, it's not very often and I have used it for years without noticing, but I suspect it might've always been possible for it to happen.

System.InvalidOperationException: Collection was modified; enumeration operation may not execute. at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource) at System.Collections.Generic.Dictionary<T1,T2>.ValueCollection.Enumerator.MoveNext() at React.JavaScriptEngineFactory.GetFactory(JsEngineSwitcher jsEngineSwitcher, Boolean allowMsie) at React.JavaScriptEngineFactory..ctor(JsEngineSwitcher jsEngineSwitcher, IReactSiteConfiguration config, IFileSystem fileSystem)

From looking at the code this is probably the culprit (line 261 of JavascriptEngineFactory) foreach (var engineFactory in jsEngineSwitcher.EngineFactories){ ... }

Now I'm not sure at all why the set of jsEngineSwitcher.EngineFactories is modified, but that does seem to be what happens. I suspect the same issue is present in later version (I'm still on 3.2.0 in most projects), since that part of the code hasn't changed from what I can see. Should be fairly simple to fix by copying the engines to a new collection before looping over them (can PR if you want), or do you think that could introduce other bugs?

Full stack trace:


System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary.ValueCollection.Enumerator.MoveNext()
   at React.JavaScriptEngineFactory.GetFactory(JsEngineSwitcher jsEngineSwitcher, Boolean allowMsie)
   at React.JavaScriptEngineFactory..ctor(JsEngineSwitcher jsEngineSwitcher, IReactSiteConfiguration config, IFileSystem fileSystem)
   at MN.React.ReactJavascriptEngineFactory.b__8_0(Int32 _langaugeId)
   at System.Collections.Concurrent.ConcurrentDictionary.GetOrAdd(TKey key, Func valueFactory)
   at MN.React.ReactJavascriptEngineFactory.GetEngine()
   at System.Lazy.CreateValue()
   at System.Lazy.LazyInitValue()
   at React.ReactEnvironment.get_Engine()
   at React.ReactEnvironment.EnsureUserScriptsLoaded()
   at React.ReactEnvironment.CreateComponent[T](String componentName, T props, String containerId, Boolean clientOnly)
   at React.Web.Mvc.HtmlHelperExtensions.React[T](HtmlHelper htmlHelper, String componentName, T props, String htmlTag, String containerId, Boolean clientOnly, Boolean serverOnly, String containerClass)
   at MN.Webpack.AspNet.HtmlHelperExtensions.React[T](HtmlHelper helper, String entry, T props, String htmlTag, String containerId, Boolean clientOnly, Boolean serverOnly, String containerClass)
   at ASP._Page_Areas_mvc_Views_List_List_cshtml.Execute() in D:\Octopus\Applications\Live\LegaOnline.master\1.0.6977.21538\Areas\mvc\Views\List\List.cshtml:line 34
   at System.Web.WebPages.WebPageBase.ExecutePageHierarchy()
   at System.Web.Mvc.WebViewPage.ExecutePageHierarchy()
   at System.Web.WebPages.StartPage.ExecutePageHierarchy()
   at System.Web.WebPages.WebPageBase.ExecutePageHierarchy(WebPageContext pageContext, TextWriter writer, WebPageRenderingBase startPage)
   at System.Web.Mvc.ViewResultBase.ExecuteResult(ControllerContext context)
   at System.Web.Mvc.ControllerActionInvoker.InvokeActionResultFilterRecursive(IList filters, Int32 filterIndex, ResultExecutingContext preContext, ControllerContext controllerContext, ActionResult actionResult)
   at System.Web.Mvc.ControllerActionInvoker.InvokeActionResultFilterRecursive(IList filters, Int32 filterIndex, ResultExecutingContext preContext, ControllerContext controllerContext, ActionResult actionResult)
   at System.Web.Mvc.ControllerActionInvoker.InvokeActionResultWithFilters(ControllerContext controllerContext, IList filters, ActionResult actionResult)
   at System.Web.Mvc.Async.AsyncControllerActionInvoker.<>c__DisplayClass21.b__1e(IAsyncResult asyncResult)
   at System.Web.Mvc.Async.AsyncControllerActionInvoker.EndInvokeAction(IAsyncResult asyncResult)
   at System.Web.Mvc.Controller.b__1d(IAsyncResult asyncResult, ExecuteCoreState innerState)
   at System.Web.Mvc.Async.AsyncResultWrapper.WrappedAsyncVoid.CallEndDelegate(IAsyncResult asyncResult)
   at System.Web.Mvc.Controller.EndExecuteCore(IAsyncResult asyncResult)
   at System.Web.Mvc.Async.AsyncResultWrapper.WrappedAsyncVoid.CallEndDelegate(IAsyncResult asyncResult)
   at System.Web.Mvc.Controller.EndExecute(IAsyncResult asyncResult)
   at System.Web.Mvc.MvcHandler.b__5(IAsyncResult asyncResult, ProcessRequestState innerState)
   at System.Web.Mvc.Async.AsyncResultWrapper.WrappedAsyncVoid.CallEndDelegate(IAsyncResult asyncResult)
   at System.Web.Mvc.MvcHandler.EndProcessRequest(IAsyncResult asyncResult)
   at System.Web.HttpApplication.CallHandlerExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute()
   at System.Web.HttpApplication.ExecuteStepImpl(IExecutionStep step)
   at System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously)

dustinsoftware commented 5 years ago

I'd like to dig further into why EngineFactories is getting modified. Without a repro it's not safe to say whether the fix would work or not.

How often is "every now and then"? Would you be willing to test out a debug build in production with your fix to see if it resolves the issue (assuming we can't find the cause for certain)

LinusCenterstrom commented 5 years ago

It happens about once or twice a week in applications with hundreds of daily users. I'm not able to test with a debug build in production I'm afraid.

Taritsyn commented 5 years ago

@LinusCenterstrom In this case, you need to look in the source code of your application where the EngineFactories collection changes. Did you use some other libraries that use the JavaScript Engine Switcher?

Taritsyn commented 5 years ago

@Daniel15 @dustinsoftware To prevent similar errors in the future, I can add additional members to the JsEngineFactoryCollection class (by analogy with the BundleCollection class from the Microsoft ASP.NET Web Optimization Framework):

…
/// <summary>
/// Gets a number of <see cref="IJsEngineFactory"/> objects in the collection
/// </summary>
public int Count
{
    get { return _factories.Count; }
}

/// <summary>
/// Gets all registered factories
/// </summary>
/// <returns>A read-only collection of all <see cref="IJsEngineFactory"/> objects in the collection</returns>
public ReadOnlyCollection<IJsEngineFactory> GetRegisteredFactories()
{
    return new List<IJsEngineFactory>(_factories.Values).AsReadOnly();
}
…

Also сan think about implementing the ICollection and ICollection<T> interfaces, but perhaps this will be superfluous.

LinusCenterstrom commented 5 years ago

The error likely occurs when the application pool is recycled. I do not use any other packages that use the JavaScript Engine Switcher as far as I know

Taritsyn commented 5 years ago

The error likely occurs when the application pool is recycled.

In old versions of the JavaScript Engine Switcher there were errors that occurred during the finalization of JS engines (“Finalazier thread is blocked becasue of JavaScriptEngineSwitcher.ChakraCore.ChakraCoreJsEngine” and “Block finalizer solved?”).

Therefore, I recommend to update the JavaScript Engine Switcher to the latest compatible versions:

Update-Package JavaScriptEngineSwitcher.Core -Version 2.4.10
Update-Package JavaScriptEngineSwitcher.ChakraCore -Version 2.4.29
Update-Package JavaScriptEngineSwitcher.Msie -Version 2.4.29
Update-Package JavaScriptEngineSwitcher.V8 -Version 2.4.13
Update-Package JavaScriptEngineSwitcher.V8.Native.win-x64 -Version 2.4.16
Update-Package MsieJavaScriptEngine -Version 2.2.10

I also recommend that you explicitly specify the default JS engine.

dustinsoftware commented 5 years ago

Going to close this one out since JS engine factories are not automatically registered in 4.x anymore. Check out the getting started for ASP.NET page for more details. Since engine factories are registered at app startup you shouldn't have this problem, but feel free to ping me on this thread if you do.

reactjs / React.NET