370231 - Constructing RegExp instance fails if standard objects scope has been sealed

Status: RESOLVED DUPLICATE of bug 319614

(Rhino Graveyard :: Core, defect)

Description

[Matt Whitlock]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 2.0.50727; .NET CLR 1.1.4322)
Build Identifier: 

If the standard objects scope has been sealed prior to accessing the RegExp class for the first time, it is impossible to construct a RegExp instance.

Reproducible: Always

Steps to Reproduce:
ScriptableObject scope = cx.initStandardObjects(null, true);
scope.sealObject();
cx.evaluateString(scope, "/foo/", "", 0, null); // exception thrown
Actual Results:  
org.mozilla.javascript.EvaluatorException: Cannot modify a property of a sealed object: RegExp.
	at org.mozilla.javascript.DefaultErrorReporter.runtimeError(DefaultErrorReporter.java:98)
	at org.mozilla.javascript.Context.reportRuntimeError(Context.java:966)
	at org.mozilla.javascript.Context.reportRuntimeError(Context.java:1022)
	at org.mozilla.javascript.Context.reportRuntimeError1(Context.java:985)
	at org.mozilla.javascript.ScriptableObject.put(ScriptableObject.java:234)
	at org.mozilla.javascript.IdScriptableObject.put(IdScriptableObject.java:420)
	at org.mozilla.javascript.ScriptableObject.defineProperty(ScriptableObject.java:1003)
	at org.mozilla.javascript.ScriptableObject.defineProperty(ScriptableObject.java:1022)
	at org.mozilla.javascript.regexp.NativeRegExp.init(NativeRegExp.java:168)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at org.mozilla.javascript.ScriptableObject.defineClass(ScriptableObject.java:814)
	at org.mozilla.javascript.ScriptableObject.defineClass(ScriptableObject.java:765)
	at org.mozilla.javascript.LazilyLoadedCtor.getProperty(LazilyLoadedCtor.java:77)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at org.mozilla.javascript.MemberBox.invoke(MemberBox.java:145)
	at org.mozilla.javascript.ScriptableObject.getByGetter(ScriptableObject.java:1677)
	at org.mozilla.javascript.ScriptableObject.get(ScriptableObject.java:180)
	at org.mozilla.javascript.IdScriptableObject.get(IdScriptableObject.java:386)
	at org.mozilla.javascript.ScriptableObject.getProperty(ScriptableObject.java:1315)
	at org.mozilla.javascript.ScriptableObject.getClassPrototype(ScriptableObject.java:1235)
	at org.mozilla.javascript.ScriptRuntime.setObjectProtoAndParent(ScriptRuntime.java:3022)
	at org.mozilla.javascript.regexp.NativeRegExp.<init>(NativeRegExp.java:175)
	at org.mozilla.javascript.regexp.RegExpImpl.wrapRegExp(RegExpImpl.java:59)
	at org.mozilla.javascript.gen.c11._c0()
	at org.mozilla.javascript.gen.c11.call()
	at org.mozilla.javascript.ContextFactory.doTopCall(ContextFactory.java:340)
	at org.mozilla.javascript.ScriptRuntime.doTopCall(ScriptRuntime.java:2758)
	at org.mozilla.javascript.gen.c11.call()
	at org.mozilla.javascript.gen.c11.exec()
	at org.mozilla.javascript.Context.evaluateString(Context.java:1132)


Expected Results:  
The expression /foo/ should evaluate to a new RegExp instance.

This bug can be worked around by referencing the RegExp class before sealing the standard objects scope:

ScriptableObject scope = cx.initStandardObjects(null, true);
cx.evaluateString(scope, "RegExp", "", 0, null); // <-- workaround
scope.sealObject();
cx.evaluateString(scope, "/foo/", "", 0, null); // no exception

This bug is similar in nature to bug #319614.

Comment 1

[Attila Szegedi]

> This bug is similar in nature to bug #319614.

Indeed - my first thought was the same. Do you have an idea how would you like to see this fixed? My opinion is that lazy loading and sealed scopes don't mix well. Actually, they're typically just the opposite use cases:
- lazy loading is good to avoid initialization performance hit when you're creating lots of top level scopes with standard objects (and don't reuse the scopes)
- sealing is good to avoid execution of one program to affect execution of another by modifying the top level scope (and reusing it).

A rather trivial fix for this would be to force evaluation of all LazilyLoadedCtors within sealObject(). I'd like to hear other people's opinion about this first, though.

BTW, a much less expensive (in CPU usage terms) workaround is to use

ScriptableObject.getProperty(scope, "RegExp");

Comment 2

[Matt Whitlock]

Thanks for the cheaper workaround.

I would be okay with it evaluating all the LazilyLoadedCtors during sealing.  We only create one scope that we initialize the standard objects on, and we seal it, and then we use that one scope throughout the life of the application.  (Of course, we create additional scopes that have that "global library scope" set as their prototype, but those additional scopes are cheap since they're essentially empty until the user puts some variables in them.)  Since we only do the sealObject once, it would be fine if it took a performance hit to make sure everything was good to be sealed.

If you do proceed with your proposed "trivial fix," you should add a note to the documentation for sealObject() that performing it on a standard object scope is a relatively expensive operation, so it should only be done when the scope is going to be reused many times.