Add support for isolated execution context

[lahma]

This seems to work now quite nicely, at least as a proof of concept. A disposable context can be created which allows introducing variables and whatnot and when context is disposed they disappear. Also sets everything resolved from outer context (say the real global) as read-only and throws errors if you try to do something like Math.max = null;.

Here's the code sample from test case demonstrating the functionality:

var engine = new Engine(options => options.Strict());
engine.SetValue("assert", new Action<bool>(Assert.True));

// Set outer variable in global scope
engine.SetValue("outer", 123);
engine.Execute("assert(outer === 123)");
engine.GetValue("outer").ToObject().Should().Be(123);

// Enter new execution context
using (engine.EnterIsolatedContext())
{
	// Can see global scope
	engine.Execute("assert(outer === 123)");

	// Can modify global scope
	engine.Execute("outer = 321");
	engine.GetValue("outer").ToObject().Should().Be(321);

	// Create variable in new context
	engine.Execute("var inner = 456");
	var value = engine.Execute("inner").GetCompletionValue();
	engine.Execute("assert(inner === 456)");

	// cannot break anything
	Assert.Throws<NotSupportedException>(() => engine.Execute("Math.max = null;"));
}

// The new variable is no longer visible
engine.Execute("assert(typeof inner === 'undefined')");

// The new variable is not in the global scope
engine.GetValue("inner").ToObject().Should().BeNull();

var max = engine.Execute("Math.max").GetCompletionValue();
max.ToObject().Should().NotBeNull();

// but can again break things
engine.Execute("Math.max = null;");
engine.GetValue("Math.max").ToObject().Should().BeNull();

When coupled with default engine pool implementation (to be done) this should allow the common pattern people request, a pre-warmed engine with scripts that can then be called with temporary catching context that is cleaned after the call.

Relates to

• Change in behaviour when calling the Engine.EnterExecutionContext method #756
• Is an object pool suggested for speed performance? #695
• Is it possible to clone or save/restore engine state? #491

fixes #738
fixes #805

[sebastienros]

Should we be able to use this without the dispose pattern, such that we can also do "LeaveIsolatedContext", so the engine would store the context and dispose it when it's called.

Might be easier to make the pool implementation.

[sebastienros]

I meant to keep the Dispose pattern, just that also being able to call Leave, that will invoke Dispose on the local context.

[sebastienros]

How is engine.Execute("outer = 321"); in the scope different from updating Math.max ?

[sebastienros]

What is the value when the scope is released?
Why doesn't it throw an exception like Math.max. Not saying it should, but how do we know it won't?

[lahma]

I explained this below wrt Math.max, TLDR; we can rewrite and modify anything that can be attached to our isolated context - this is shadowing vars. outer = 32; is effectively var outer = 321; so introducing a new variable isolated environment hiding old from global.

[sebastienros]

Size should be the last argument.
It should not be mandatory.

[sebastienros]

Isn't this method already thread-safe ?

[lahma]

Well they used to be 😉 When I brought the pool from MS pool package, we traded thread-safety for speed as Jint is not thread-safe. Would require second impl with the origimal Interlocked functionality.

[sebastienros]

Maybe create an IEngine instead that is implemented by Engine too. And the pooled version could handle the dispose pattern, and have a Release/Return() method if we don't want to use the dispose pattern.

[lahma]

Can do, I just want to IEngine to strictly limited to execution and setting values to context. Might be role interface too. Just need to limit public access to things like ObjectConstructor etc which are now public and might allow doing bad things.

[lahma]

I think I'll stick for wrapper now so that we can implement the dispose and checks here. Or should the Engine implement it as quite no-op for now and we would just start checking if (_disposed) { throw diposedexception} in engine service methods?

[sebastienros]

With the interface, the pool could return an IEngine directly, which would be the result of this call.

[lahma]

How is engine.Execute("outer = 321"); in the scope different from updating Math.max ?

This is a bit tricky and probably counter-intuitive at first. When you update anything var-like which is bound to global scope (Math, outer) we can easily catch that as it comes as request to our IsolatedEnvironmentRecord. This is the key to this shadowing and isolation in my approach - environment record tracks sets and protects original record and Global object.

But when you do Math.max = null; it first resolves the target object and then calls Set logic for the ObjectInstance, so "get Math instance m and call m.Set("max", JsNull);. This is why we wrap anything resolved as object instance as read-only which is resolved from outer scope (the real global environment).

So key difference is that doing var Math = ({}); Math,max = () => 42; Introduces new var to our isolated context and we can allow altering it however one wants, trying to alter real global's sub-properties will poison the engine as we cannot easily track all the object graphs and restore them later.

[lahma]

Now we have support for ActivateTemporaryConstraints which makes me feel that should we group these under same prefix, like ActivateIsolatedContext which then both follow IDisposable semantics. I also feel inclined to remove the EnterExecutionContext and LeaveExecutionContext for public API, I know some have been using them but I guess they are quite complex and broken for the old use cases after let/const which introuced new concepts.

The pool API also needs some love. Allocate and Free are there and now return the full Engine which allows you to do bad things... But I think those are for expert users who want to track life time and cleanup themselves..

[sebastienros]

I don't see the point of IEngine if we have IScriptExecutor. These two methods in IEngine are used by the EngineWrapper, and it knows about Engine, so the interface seems useless. At least based on the fact that I don't see a user accessing them. Or I would just shift the interface names to

• IEngine (instead of IScriptExecutor)
• IActivatedEngine (instead of IEngine), if you have a reason to keep this interface.

Can you share the results of the benchmarks?

[lahma]

I think this PR is coming a bit of overwhelmed with all the needs. My original thinking was to clean up Engine by providing IEngine that would only allow moderate amount of functionality, but that might be a long shot. Currently Engine exposes a lot of things and they make it hard to be backwards compatible, like EnterExecutionContext - the thing we are trying to remedy here. IEngine should probably only offer functionality somewhat described in spec (constructors, global objects).

Maybe we need to think about having the IEngine that exposes things JS has like Global and Array.Prototype and such that are set in stone. The pooled engine needs to have more isolation and try to hide some things to reduce possibility of "state contamination".

There are a lot of things here, like why we expose PrototypeObject instead of Prototype which would make more sense in JS kind of way.

[sebastienros]

I like it better. So anything that can be isolated should be in the interface that is returned by isolated pools, e.g. IIsolatedEnginePool -> IsolatedEngine and anything that can't be should be in other factories, like DefaultEngineFactory -> IEngine. And if we don't know which properties can't be isolated, then keep a single, minimal interface returned from pools, and let users use the DefaultEngineFactory to get Engine instances that can be manipulated without no assumptions about the engine being reused.

[lahma]

Changed some names again, here's the results you asked:

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.572 (2004/?/20H1)
AMD Ryzen 7 2700X, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=5.0.100-rc.2.20479.15
  [Host]     : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  DefaultJob : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
RunUsingPooledIsolated	777.3 ms	7.50 ms	6.65 ms	23000.0000	1000.0000	-	95.32 MB
RunUsingDirtiedEngineInstance	777.9 ms	4.03 ms	3.15 ms	23000.0000	1000.0000	-	95.32 MB
RunUsingNewEngineInstance	2,517.1 ms	48.53 ms	57.77 ms	150000.0000	28000.0000	18000.0000	1954.5 MB

[KurtGokhan]

I don't know how much of this is done and already working, but I wanted to mention a new ES proposal that has quite similar features. It is called Realms API. It is a stage 2 proposal and is subject to change. But I believe it will make it way into the standard as it is a commonly needed pattern in Javascript engines. So if you want to have an in-engine solution, you may consider Realms API.

[lahma]

I was actually reading the specs just today! I'm investigating that API as a way to upgrade AngleSharp.JS to Jint 3 as they want window as global. So far it seems that Realms is only little bit lighter than new engine instance, here this PR tries to allow existing constructs to be reused. Probably needs more investigation. Thanks for the interest, I'm open to ideas and usage scenarios you have in mind!

[ejsmith]

This looks really awesome!! What are the chances of this PR landing any time soon?

[lahma]

I think we should get initial version of realms/host support first in. This will allow creating a custom host for isolated context (like a browser has) that can inject custom global and participate in execution context creation.

[ejsmith]

Hmm... that sounds like it's going to be a long ways off. :-(

[sebastienros]

@ejsmith what scenario is really important for you that is covered in this PR?

[ejsmith]

We have a multi-tenanted app that uses JS for customization and we pool engine instances but I'm worried that the global scope could be altered and leak to other tenants.

[lahma]

Sharing a pool between tenants can be troublesome indeed, just needs a code like this to run Math.max = () => 1; Math.max(2, 3);, prints 1. And that was just a little prank, worse things are of course possible.

For what's it worth, we did improve the engine creation performance a while back, but if you have a lot of initialization going on might not suffice for your use case.

[lahma]

Created #907 for the preliminary work.

[iXyles]

Hello,

Out of curiosity, are there any plans for when this might become available?

[lahma]

I think we've created a Duke Nukem project here, let's see when it's ready.

[lahma]

ShadowRealm was merged to main and it might contain the functionality aimed in this PR, see Jint test case how it isolates a playground from the actual engine.

[lahma]

I think ShadowRealm fulfils the purpose well enough and is a standard way to handle this. I'll close this experiment for now.

[gentledepp]

thanks for the update, @lahma.
Just one question: I cannot find the jint test case anymore. Was that one removed?

[lahma]

@gentledepp it was moved to public API test project: https://github.com/sebastienros/jint/blob/main/Jint.Tests.PublicInterface/ShadowRealmTests.cs

[gentledepp]

Thank you so much! This is awesome!