# HG changeset patch # User attila # Date 1432726631 -10800 # Node ID ecffc563dfcf526e22b1bc65fa55e806c020d1ea # Parent 7ffaac79bd34a8ce58e8e1a45245cb77f1edcc95 8081204: ListAdapter throws NPE when adding/removing elements outside of JS context Reviewed-by: lagergren, sundar diff -r 7ffaac79bd34 -r ecffc563dfcf nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ListAdapter.java --- 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 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 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 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 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 creator) { - return Context.getGlobal().getDynamicInvoker(key, creator); + private MethodHandle getDynamicInvoker(final Object key, final Callable creator) { + return global.getDynamicInvoker(key, creator); } } diff -r 7ffaac79bd34 -r ecffc563dfcf nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/Messages.properties --- 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. diff -r 7ffaac79bd34 -r ecffc563dfcf nashorn/test/src/jdk/nashorn/internal/runtime/test/AddAndRemoveOnListAdapterOutsideOfJavaScriptContextTest.java --- /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 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 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 l = getListAdapter(); + assertEquals(l.removeLast(), 4); + assertEquals(l.size(), 3); + assertEquals(l.getLast(), 3); + } + + @Test + public void testUnshift() throws ScriptException { + final Deque 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 l = getListAdapter(); + l.removeFirst(); + assertEquals(l.getFirst(), 2); + assertEquals(l.getLast(), 4); + assertEquals(l.size(), 3); + } + + @Test + public void testSpliceAdd() throws ScriptException { + final List 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 l = getListAdapter(); + assertEquals(l, Arrays.asList(1, 2, 3, 4)); + l.remove(2); + assertEquals(l, Arrays.asList(1, 2, 4)); + } +}