changeset 30834:ecffc563dfcf

8081204: ListAdapter throws NPE when adding/removing elements outside of JS context Reviewed-by: lagergren, sundar
author attila
date Wed, 27 May 2015 14:37:11 +0300
parents 7ffaac79bd34
children 60b49c9d8280
files nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ListAdapter.java nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/Messages.properties nashorn/test/src/jdk/nashorn/internal/runtime/test/AddAndRemoveOnListAdapterOutsideOfJavaScriptContextTest.java
diffstat 3 files changed, 127 insertions(+), 34 deletions(-) [+]
line wrap: on
line diff
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ListAdapter.java	Wed May 27 13:16:50 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ListAdapter.java	Wed May 27 14:37:11 2015 +0300
@@ -36,6 +36,7 @@
 import java.util.concurrent.Callable;
 import jdk.nashorn.api.scripting.JSObject;
 import jdk.nashorn.api.scripting.ScriptObjectMirror;
+import jdk.nashorn.internal.objects.Global;
 import jdk.nashorn.internal.runtime.linker.Bootstrap;
 
 /**
@@ -55,53 +56,43 @@
     // Invoker creator for methods that add to the start or end of the list: PUSH and UNSHIFT. Takes fn, this, and value, returns void.
     private static final Callable<MethodHandle> ADD_INVOKER_CREATOR = invokerCreator(void.class, Object.class, JSObject.class, Object.class);
 
-    // PUSH adds to the end of the list
+    // PUSH adds to the start of the list
     private static final Object PUSH = new Object();
-    private static MethodHandle getPushInvoker() {
-        return getDynamicInvoker(PUSH, ADD_INVOKER_CREATOR);
-    }
-
-    // UNSHIFT adds to the start of the list
+    // UNSHIFT adds to the end of the list
     private static final Object UNSHIFT = new Object();
-    private static MethodHandle getUnshiftInvoker() {
-        return getDynamicInvoker(UNSHIFT, ADD_INVOKER_CREATOR);
-    }
 
     // Invoker creator for methods that remove from the tail or head of the list: POP and SHIFT. Takes fn, this, returns Object.
     private static final Callable<MethodHandle> REMOVE_INVOKER_CREATOR = invokerCreator(Object.class, Object.class, JSObject.class);
 
-    // POP removes from the to the end of the list
+    // POP removes from the start of the list
     private static final Object POP = new Object();
-    private static MethodHandle getPopInvoker() {
-        return getDynamicInvoker(POP, REMOVE_INVOKER_CREATOR);
-    }
-
-    // SHIFT removes from the to the start of the list
+    // SHIFT removes from the end of the list
     private static final Object SHIFT = new Object();
-    private static MethodHandle getShiftInvoker() {
-        return getDynamicInvoker(SHIFT, REMOVE_INVOKER_CREATOR);
-    }
 
     // SPLICE can be used to add a value in the middle of the list.
     private static final Object SPLICE_ADD = new Object();
     private static final Callable<MethodHandle> SPLICE_ADD_INVOKER_CREATOR = invokerCreator(void.class, Object.class, JSObject.class, int.class, int.class, Object.class);
-    private static MethodHandle getSpliceAddInvoker() {
-        return getDynamicInvoker(SPLICE_ADD, SPLICE_ADD_INVOKER_CREATOR);
-    }
 
     // SPLICE can also be used to remove values from the middle of the list.
     private static final Object SPLICE_REMOVE = new Object();
     private static final Callable<MethodHandle> SPLICE_REMOVE_INVOKER_CREATOR = invokerCreator(void.class, Object.class, JSObject.class, int.class, int.class);
-    private static MethodHandle getSpliceRemoveInvoker() {
-        return getDynamicInvoker(SPLICE_REMOVE, SPLICE_REMOVE_INVOKER_CREATOR);
-    }
 
     /** wrapped object */
-    protected final JSObject obj;
+    private final JSObject obj;
+    private final Global global;
 
     // allow subclasses only in this package
     ListAdapter(final JSObject obj) {
         this.obj = obj;
+        this.global = getGlobalNonNull();
+    }
+
+    private static Global getGlobalNonNull() {
+        final Global global = Context.getGlobal();
+        if (global != null) {
+            return global;
+        }
+        throw new IllegalStateException(ECMAErrors.getMessage("list.adapter.null.global"));
     }
 
     /**
@@ -166,7 +157,7 @@
     @Override
     public final void addFirst(final Object e) {
         try {
-            getUnshiftInvoker().invokeExact(getFunction("unshift"), obj, e);
+            getDynamicInvoker(UNSHIFT, ADD_INVOKER_CREATOR).invokeExact(getFunction("unshift"), obj, e);
         } catch(RuntimeException | Error ex) {
             throw ex;
         } catch(final Throwable t) {
@@ -177,7 +168,7 @@
     @Override
     public final void addLast(final Object e) {
         try {
-            getPushInvoker().invokeExact(getFunction("push"), obj, e);
+            getDynamicInvoker(PUSH, ADD_INVOKER_CREATOR).invokeExact(getFunction("push"), obj, e);
         } catch(RuntimeException | Error ex) {
             throw ex;
         } catch(final Throwable t) {
@@ -195,7 +186,7 @@
             } else {
                 final int size = size();
                 if(index < size) {
-                    getSpliceAddInvoker().invokeExact(obj.getMember("splice"), obj, index, 0, e);
+                    getDynamicInvoker(SPLICE_ADD, SPLICE_ADD_INVOKER_CREATOR).invokeExact(obj.getMember("splice"), obj, index, 0, e);
                 } else if(index == size) {
                     addLast(e);
                 } else {
@@ -287,7 +278,7 @@
 
     private Object invokeShift() {
         try {
-            return getShiftInvoker().invokeExact(getFunction("shift"), obj);
+            return getDynamicInvoker(SHIFT, REMOVE_INVOKER_CREATOR).invokeExact(getFunction("shift"), obj);
         } catch(RuntimeException | Error ex) {
             throw ex;
         } catch(final Throwable t) {
@@ -297,7 +288,7 @@
 
     private Object invokePop() {
         try {
-            return getPopInvoker().invokeExact(getFunction("pop"), obj);
+            return getDynamicInvoker(POP, REMOVE_INVOKER_CREATOR).invokeExact(getFunction("pop"), obj);
         } catch(RuntimeException | Error ex) {
             throw ex;
         } catch(final Throwable t) {
@@ -312,7 +303,7 @@
 
     private void invokeSpliceRemove(final int fromIndex, final int count) {
         try {
-            getSpliceRemoveInvoker().invokeExact(getFunction("splice"), obj, fromIndex, count);
+            getDynamicInvoker(SPLICE_REMOVE, SPLICE_REMOVE_INVOKER_CREATOR).invokeExact(getFunction("splice"), obj, fromIndex, count);
         } catch(RuntimeException | Error ex) {
             throw ex;
         } catch(final Throwable t) {
@@ -417,7 +408,7 @@
         };
     }
 
-    private static MethodHandle getDynamicInvoker(final Object key, final Callable<MethodHandle> creator) {
-        return Context.getGlobal().getDynamicInvoker(key, creator);
+    private MethodHandle getDynamicInvoker(final Object key, final Callable<MethodHandle> creator) {
+        return global.getDynamicInvoker(key, creator);
     }
 }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/Messages.properties	Wed May 27 13:16:50 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/Messages.properties	Wed May 27 14:37:11 2015 +0300
@@ -174,4 +174,4 @@
 config.error.no.dest=no destination directory supplied
 
 uri.error.bad.uri=Bad URI "{0}" near offset {1}
-
+list.adapter.null.global=Attempted to create the adapter from outside a JavaScript execution context.
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/src/jdk/nashorn/internal/runtime/test/AddAndRemoveOnListAdapterOutsideOfJavaScriptContextTest.java	Wed May 27 14:37:11 2015 +0300
@@ -0,0 +1,102 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package jdk.nashorn.internal.runtime.test;
+
+import static org.testng.Assert.assertEquals;
+
+import java.util.Arrays;
+import java.util.Deque;
+import java.util.List;
+import javax.script.ScriptEngine;
+import javax.script.ScriptException;
+import jdk.nashorn.api.scripting.NashornScriptEngineFactory;
+import org.testng.annotations.Test;
+
+/**
+ * @bug 8081204
+ * @summary adding and removing elements to a ListAdapter outside of JS context should work.
+ */
+@SuppressWarnings("javadoc")
+public class AddAndRemoveOnListAdapterOutsideOfJavaScriptContextTest {
+
+    @SuppressWarnings("unchecked")
+    private static <T> T getListAdapter() throws ScriptException {
+        final ScriptEngine engine = new NashornScriptEngineFactory().getScriptEngine();
+        return (T)engine.eval("Java.to([1, 2, 3, 4], 'java.util.List')");
+    }
+
+    @Test
+    public void testInvokePush() throws ScriptException {
+        final Deque<Object> l = getListAdapter();
+        l.addLast(5);
+        assertEquals(l.size(), 5);
+        assertEquals(l.getLast(), 5);
+        assertEquals(l.getFirst(), 1);
+    }
+
+    @Test
+    public void testPop() throws ScriptException {
+        final Deque<Object> l = getListAdapter();
+        assertEquals(l.removeLast(), 4);
+        assertEquals(l.size(), 3);
+        assertEquals(l.getLast(), 3);
+    }
+
+    @Test
+    public void testUnshift() throws ScriptException {
+        final Deque<Object> l = getListAdapter();
+        l.addFirst(0);
+        assertEquals(l.getFirst(), 0);
+        assertEquals(l.getLast(), 4);
+        assertEquals(l.size(), 5);
+    }
+
+    @Test
+    public void testShift() throws ScriptException {
+        final Deque<Object> l = getListAdapter();
+        l.removeFirst();
+        assertEquals(l.getFirst(), 2);
+        assertEquals(l.getLast(), 4);
+        assertEquals(l.size(), 3);
+    }
+
+    @Test
+    public void testSpliceAdd() throws ScriptException {
+        final List<Object> l = getListAdapter();
+        assertEquals(l, Arrays.asList(1, 2, 3, 4));
+        l.add(2, "foo");
+        assertEquals(l, Arrays.asList(1, 2, "foo", 3, 4));
+    }
+
+
+    @Test
+    public void testSpliceRemove() throws ScriptException {
+        final List<Object> l = getListAdapter();
+        assertEquals(l, Arrays.asList(1, 2, 3, 4));
+        l.remove(2);
+        assertEquals(l, Arrays.asList(1, 2, 4));
+    }
+}