`Object.create` across module boundaries is broken

[szeiger]

I ran into a problem running highlight.js that took me all day to debug and work around. It can be reproduced by fully initializing the highlight.js NPM module and then trying to highlight a string containing double quotes with the Scala highlighter:

  val engine = ... // NashornScriptEngine
  val mainModule = Require.enable(engine, ...)
  val hl = mainModule.require("highlight.js")
  engine.invokeMethod(hl, "highlight", "scala", "\"foo\"")

This triggers the initialization of the Scala highlighter where Object.create(proto, ...) gets called at some point. It is called from within lib/highlight.js using a prototype object that was originally created in the module lib/languages/scala.js. For some reason this object appears as a ScriptObjectMirror within Nashorn and not as a ScriptObject. This makes Object.create fail with:

javax.script.ScriptException: TypeError: [object Object] is not an Object in <eval> at line number 540 at column number 8

This problem does not occur when intializing highlight.js manually without nashorn-commonjs-modules by loading all required files and running them through engine.eval calls with the default context.

I was able to work around the problem by inserting the following patch in the constructor of Module to replace Object.create with a version that can unwrap a ScriptObjectMirror to the underlying ScriptObject:

  engine.eval( // Patch Object.create to unwrap ScriptObjectMirror to ScriptObject
    """(function() {
      var realCreate = Object.create;
      var su = Java.type('jdk.nashorn.api.scripting.ScriptUtils');
      Object.create = function(p, v) { return realCreate(su.unwrap(p), v); };
    })();""", global)

I am still not sure if there's something wrong in Module that could be done differently to avoid the problem, or if it's simply a bug in Nashorn. Googling for the symptoms didn't yield any useful results.

[szeiger]

Testing some more cases, it turns out that the ScriptObjectMirrors can have incompatible Globals, in which case ScriptUtils.unwrap does not work, so an even bigger hack is required:

  def unwrap(o: ScriptObjectMirror): AnyRef = {
    val f = classOf[ScriptObjectMirror].getDeclaredField("sobj")
    f.setAccessible(true)
    f.get(o)
  }

[malaporte]

Thanks for the reports! I'll have a look once I can get hold of my laptop for more than a few minutes.

[malaporte]

OK this does seem to be a JDK bug somehow, although I haven't investigated it in depth. I can reproduce the problem when running JDK 1.8.0_40, but if I move up to 1.8.0_77 the code works fine.

I still find this annoying to require a specific version of Java - I'll try to understand the issue better to see if there is a possible workaround (maybe the hack you suggest would do the trick, but yeah...)

[szeiger]

FTR, this is the version where I ran into the bug:

$ java -version
java version "1.8.0_60"
Java(TM) SE Runtime Environment (build 1.8.0_60-b27)
Java HotSpot(TM) 64-Bit Server VM (build 25.60-b23, mixed mode)

Maybe I should just upgrade...

In the meantime I've had to extend my workaround to deal with null:

  def unwrap(o: ScriptObjectMirror): AnyRef = if(o eq null) null else {
    val f = classOf[ScriptObjectMirror].getDeclaredField("sobj")
    f.setAccessible(true)
    f.get(o)
  }

[malaporte]

I've been able to reproduce the issue with this minimal code:

ScriptEngine engine = new ScriptEngineManager().getEngineByName("nashorn");
engine.put("foo", engine.eval("function() { return {}; }", new SimpleBindings()));
engine.eval("Object.create(foo());");

The culprit is the new SimpleBindings() argument that is passed to eval; somehow doing this causes the value returned by the JS function to not be unwrapped properly on an older JRE. That's annoying because I'm using this to have each module evaluate with his own "clean" global environment.

Still looking for an elegant fix...

[szeiger]

I upgraded to 8u102 and can confirm that this is partially fixed. It does not occur anymore in highlight.js but I still get this error with Object.create in a similar case in the domino package. Even with my workaround still enabled, domino fails to load:

Caused by: jdk.nashorn.internal.runtime.ECMAException: TypeError: function() {
  throw new Error("NotYetImplemented");
} is not a function
    at jdk.nashorn.internal.runtime.ECMAErrors.error(ECMAErrors.java:57)
    at jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:213)
    at jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:185)
    at jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:172)
    at jdk.nashorn.internal.objects.AccessorPropertyDescriptor.fillFrom(AccessorPropertyDescriptor.java:158)
    at jdk.nashorn.internal.runtime.ScriptObject.toPropertyDescriptor(ScriptObject.java:429)
    at jdk.nashorn.internal.runtime.ScriptObject.toPropertyDescriptor(ScriptObject.java:442)
    at jdk.nashorn.internal.runtime.ScriptObject.defineOwnProperty(ScriptObject.java:536)
    at jdk.nashorn.internal.objects.NativeObject.defineProperties(NativeObject.java:308)
    at jdk.nashorn.internal.objects.NativeObject.create(NativeObject.java:269)
    at jdk.nashorn.internal.scripts.Script$Recompilation$12$184AA$\^eval\_.L:2$create(Module.scala/eval1:8)
    at jdk.nashorn.internal.scripts.Script$9$\^eval\_.:program(domino/lib/Node.js:75)

This is somewhere down in the native Object.create call on the already unwrapped object. I suspect I suspect it is caused by a function that has not been properly unwrapped and is therefore not recognized as a function.

[szeiger]

The new version works much better. Here's a case where it still fails:

test5:

        var a = require('test6');
        function b() {};
        b.prototype = Object.create(a.prototype, {});
        // javax.script.ScriptException: TypeError: [object Object] is not an Object in test5 at line number 3

test6:

        module.exports = a;
        function a() {};
        a.prototype = Object.create(Object.prototype, {});

[szeiger]

I have switched to a JavaScript-based module scope in my codebase, as explained in the node.js docs. I updated my gist with the new version. This avoids using different global scopes and should provide the same isolation as in node.js.

[malaporte]

Thanks for the tip, I did the same thing and it now works for the last 2 tests you mentioned.