Clearscript: V8ScriptEngine.Script Property is Marked as Disposed?

[rolandus]

TL;DR
I'm getting the following exception when using separate instances of V8ScriptEngine created from the same instance of V8Runtime. The exception is only thrown when I've used one engine, disposed it, and created another.

Exception has occurred: CLR/System.IO.FileLoadException
An exception of type 'System.IO.FileLoadException' occurred in ClearScript.Core.dll but was not handled in user code
 Inner exceptions found, see $exception in variables window for more details.
 Innermost exception     System.ObjectDisposedException : Cannot access a disposed object.

Lengthy details below....

======================

I'm using ClearScript to load scripts from disk. The scripts are using ES6 imports to load their dependencies. The on-disk structure of my files doesn't follow normal node_modules conventions, so I'm using a custom DocumentLoader to resolve the import statements.

SELF-CONTAINED REPRO HERE: https://gist.github.com/rolandus/47e6232b3bf85c65ccd0d13907c8852d

My test application creates a single instance of V8Runtime, and then creates a new engine for each script execution by calling runtime.CreateScriptEngine(). My intent for doing this is to use V8's isolate/context abstractions, as discussed in https://github.com/microsoft/ClearScript/issues/102#issuecomment-467479984.

private V8ScriptEngine GetNewEngine()
{
    var engine = _runtime.CreateScriptEngine(V8ScriptEngineFlags.EnableDebugging);
    engine.DocumentSettings.Loader = _loader;
    engine.DocumentSettings.AccessFlags = DocumentAccessFlags.EnableFileLoading;
    return engine;
}

The engine is passed to a using statement, which (I believe) should have the same effect as calling dispose() on the engine when finished, as discussed in https://github.com/microsoft/ClearScript/issues/49#issuecomment-376202672.

using (var engine = GetNewEngine())
{
    // do stuff....
}

The final twist is that I'm using a combination of the engine's Script property and a ContextCallback to expose information from my JavaScript to my custom DocumentLoader. The reason I need this is that I have some dependency-mapping information in the JavaScript, and the loader needs to have access to it to be able to resolve the correct versions of dependencies. The use of the Script property was inspired by https://github.com/microsoft/ClearScript/issues/148#issuecomment-559817792

Now, this all works fine on the first script execution, but it fails on subsequent ones. The error is occurring in my ContextCallback, where it attempts to access the engine.Script property.

engine.Script.Global = engine.Script;
// .....
new DocumentInfo("main") { 
    Category = ModuleCategory.Standard,
    ContextCallback = (info) =>
    {
        var retVal = new Dictionary<string, object>();
        retVal.Add("packageInfo", engine.Script.Global.__packageInfo);  // < exception thrown here
        retVal.Add("packageNames", engine.Script.Global.__packageNames);
        retVal.Add("entryPoint", engine.Script.Global.__entryPoint);
        return retVal;
    }
},

SELF-CONTAINED REPRO HERE: https://gist.github.com/rolandus/47e6232b3bf85c65ccd0d13907c8852d

Strangely enough, when developing my repro case, I noticed that it only happens if there is more than one level of imports to follow in the code on disk. In other words, if there is only a single file with an import statement, it works fine. But if the imported file imports _another_ file, then the exception is thrown during resolution of that second import statement.

I noticed that the Script property of the engine is marshalled from some deep dark place here https://github.com/microsoft/ClearScript/blob/caf3c989df8c68cf384daee765a1db45eb2869f2/ClearScript/V8/V8ScriptEngine.cs#L940 and gets disposed here https://github.com/microsoft/ClearScript/blob/caf3c989df8c68cf384daee765a1db45eb2869f2/ClearScript/V8/V8ScriptEngine.cs#L1523 Is it possible that the same root object is getting shared between different instances of the engine, and accidentally getting disposed?

[ClearScriptLib]

Hi @rolandus,

The default document loader is a singleton that maintains a cache of loaded documents. Normally, as this cache only holds raw document content, it can be used safely across script engines to avoid redundant disk and network I/O.

You can think of this cache as a Uri-to-Document dictionary. It can operate correctly only as long as nothing within it is engine-specific. By using context callbacks that capture engine instances, you're populating the cache with engine-specific data.

Let's walk through your repro case:

• After the first iteration, the cache contains entries for "dummy.js" and "dummy2.js", both with context callbacks that refer to the first engine, which is now disposed.
• During the second iteration, the outermost module ("main") imports "dummy.js", invoking your custom loader. Script-based imports don't specify context callbacks, so your loader uses the one associated with "main". That works because you're constructing a unique "main" for each iteration. Your loader then calls the default loader to resolve "dummy.js", which the default loader retrieves from the cache, _along with the context callback from the first iteration_.
• "dummy.js" then imports "dummy2.js". Once again, because no context callback is specified, your loader uses the one associated with the requesting document ("dummy.js"). That callback attempts to access the disposed engine from the first iteration, and an exception is thrown.

There are several ways to fix this. First, you could use ScriptEngine.Current to decouple your context callback from a specific engine instance:

``` C#
retVal.Add("packageInfo", ScriptEngine.Current.Script.Global.__packageInfo);
retVal.Add("packageNames", ScriptEngine.Current.Script.Global.__packageNames);
retVal.Add("entryPoint", ScriptEngine.Current.Script.Global.__entryPoint);

Another possibility, albeit a drastic one, is to clear the cache as necessary:

``` C#
public async override Task<Document> LoadDocumentAsync( [...] ) {
    [...]
    Default.DiscardCachedDocuments();
    return await Default.LoadDocumentAsync(settings, sourceInfo, specifier, category, callback);
}

You could also avoid caching altogether by using a custom document loader that doesn't rely on the default.

EDIT: By the way, this may apply only to your simplified repro and not your actual application, but there's really no need to use the context callback here. That callback is intended to allow the host to provide context for scripts, not the other way around. Your custom document loader could easily bind to the engine via ScriptEngine.Current and extract whatever data it needs to do its job.

Good luck!

[rolandus]

@ClearScriptLib, that definitely explains the behavior I was seeing. Thanks for taking such a detailed look at my repro and doing the walkthrough!

Your intuition is correct that (for now), I don't need to use the context callback at all. Getting a reference to the current engine by using ScriptEngine.Current is exactly what I needed.

Keep up the good work!