You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by he...@apache.org on 2019/12/02 16:10:23 UTC

[commons-jexl] branch master updated (4f1a6d1 -> 53e5e77)

This is an automated email from the ASF dual-hosted git repository.

henrib pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git.


    from 4f1a6d1  JEXL-307: handle top-level lambda creation consistently Task #JEXL-307 - Variable redeclaration option
     new a4c4cc9  JEXL-317: added optional interface JexlContext.CancellationHandle Task #JEXL-317 - Support script cancellation through less invasive API
     new 53e5e77  JEXL-307: fixed lexical feature handling of variable scope in for/lambda units Task #JEXL-307 - Variable redeclaration option

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 RELEASE-NOTES.txt                                  |  1 +
 .../java/org/apache/commons/jexl3/JexlContext.java | 15 ++++++
 .../apache/commons/jexl3/internal/Interpreter.java |  3 +-
 .../commons/jexl3/internal/InterpreterBase.java    | 18 ++++---
 .../commons/jexl3/parser/ASTForeachStatement.java  | 62 ----------------------
 .../apache/commons/jexl3/parser/JexlParser.java    | 44 +++++++--------
 .../org/apache/commons/jexl3/parser/Parser.jjt     | 15 +++---
 src/site/xdoc/changes.xml                          |  3 ++
 .../java/org/apache/commons/jexl3/LexicalTest.java | 14 +++++
 .../apache/commons/jexl3/ScriptCallableTest.java   | 51 +++++++++++++++++-
 10 files changed, 123 insertions(+), 103 deletions(-)
 delete mode 100644 src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java


[commons-jexl] 01/02: JEXL-317: added optional interface JexlContext.CancellationHandle Task #JEXL-317 - Support script cancellation through less invasive API

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git

commit a4c4cc996b1405e1d93f8c59d0d4c3670aefffd2
Author: henrib <he...@apache.org>
AuthorDate: Mon Dec 2 17:08:09 2019 +0100

    JEXL-317: added optional interface JexlContext.CancellationHandle
    Task #JEXL-317 - Support script cancellation through less invasive API
---
 RELEASE-NOTES.txt                                  |  1 +
 .../java/org/apache/commons/jexl3/JexlContext.java | 15 +++++++
 .../apache/commons/jexl3/internal/Interpreter.java |  3 +-
 .../commons/jexl3/internal/InterpreterBase.java    | 18 ++++----
 src/site/xdoc/changes.xml                          |  3 ++
 .../apache/commons/jexl3/ScriptCallableTest.java   | 51 +++++++++++++++++++++-
 6 files changed, 81 insertions(+), 10 deletions(-)

diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index 6a6ee8d..23ce03f 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -48,6 +48,7 @@ What's new in 3.2:
 
 New Features in 3.2:
 ====================
+* JEXL-317:      Support script cancellation through less invasive API
 * JEXL-307:      Variable redeclaration option
 * JEXL-295:      Add unary plus operator
 * JEXL-292:      Allow specifying custom Permissions class for Uberspect to be used later by Introspector
diff --git a/src/main/java/org/apache/commons/jexl3/JexlContext.java b/src/main/java/org/apache/commons/jexl3/JexlContext.java
index 2f4d69e..7a9c828 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlContext.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlContext.java
@@ -18,6 +18,7 @@
 package org.apache.commons.jexl3;
 
 import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * Manages variables which can be referenced in a JEXL expression.
@@ -175,4 +176,18 @@ public interface JexlContext {
          */
         void processPragma(String key, Object value);
     }
+    
+    /**
+     * A marker interface of the JexlContext sharing a cancelling flag.
+     * <p>A script running in a thread can thus be notified through this reference
+     * of its cancellation through the context. It uses the same interpreter logic
+     * that reacts to cancellation and is an alternative to using callable() and/or
+     * interrupting script interpreter threads.
+     */
+    interface CancellationHandle {
+        /**
+         * @return a cancelable boolean used by the interpreter 
+         */
+        AtomicBoolean getCancellation();
+    }
 }
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
index 9c27f61..dac5b49 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -200,7 +200,8 @@ public class Interpreter extends InterpreterBase {
         } catch (JexlException.Return xreturn) {
             return xreturn.getValue();
         } catch (JexlException.Cancel xcancel) {
-            cancelled |= Thread.interrupted();
+            // cancelled |= Thread.interrupted();
+            cancelled.weakCompareAndSet(false, Thread.interrupted());
             if (isCancellable()) {
                 throw xcancel.clean();
             }
diff --git a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
index 504c1c1..3f4f2d0 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
@@ -20,6 +20,7 @@ package org.apache.commons.jexl3.internal;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
 import org.apache.commons.jexl3.JexlArithmetic;
 import org.apache.commons.jexl3.JexlContext;
 import org.apache.commons.jexl3.JexlEngine;
@@ -63,7 +64,7 @@ public abstract class InterpreterBase extends ParserVisitor {
     /** Cache executors. */
     protected final boolean cache;
     /** Cancellation support. */
-    protected volatile boolean cancelled = false;
+    protected final AtomicBoolean cancelled;
     /** Empty parameters for method matching. */
     protected static final Object[] EMPTY_PARAMS = new Object[0];
     /** The context to store/retrieve variables. */
@@ -100,6 +101,11 @@ public abstract class InterpreterBase extends ParserVisitor {
         } else {
             ns = Engine.EMPTY_NS;
         }
+        AtomicBoolean acancel = null;
+        if (this.context instanceof JexlContext.CancellationHandle) {
+            acancel = ((JexlContext.CancellationHandle) context).getCancellation();
+        }
+        this.cancelled = acancel != null? acancel : new AtomicBoolean(false);
         this.functions = jexl.functions;
         this.functors = null;
         this.operators = new Operators(this);
@@ -118,6 +124,7 @@ public abstract class InterpreterBase extends ParserVisitor {
         arithmetic = ii.arithmetic;
         cache = ii.cache;
         ns = ii.ns;
+        cancelled = ii.cancelled;
         functions = ii.functions;
         functors = ii.functors;
         operators = ii.operators;
@@ -544,12 +551,7 @@ public abstract class InterpreterBase extends ParserVisitor {
      * @return false if already cancelled, true otherwise
      */
     protected  boolean cancel() {
-        if (cancelled) {
-            return false;
-        } else {
-            cancelled = true;
-            return true;
-        }
+        return cancelled.compareAndSet(false, true);
     }
 
     /**
@@ -557,7 +559,7 @@ public abstract class InterpreterBase extends ParserVisitor {
      * @return true if cancelled, false otherwise
      */
     protected boolean isCancelled() {
-        return cancelled | Thread.currentThread().isInterrupted();
+        return cancelled.get() | Thread.currentThread().isInterrupted();
     }
 
     /**
diff --git a/src/site/xdoc/changes.xml b/src/site/xdoc/changes.xml
index 24e4b2a..7ecbf0c 100644
--- a/src/site/xdoc/changes.xml
+++ b/src/site/xdoc/changes.xml
@@ -26,6 +26,9 @@
     </properties>
     <body>
         <release version="3.2" date="unreleased">
+            <action dev="henrib" type="add" issue="JEXL-317">
+                Support script cancellation through less invasive API
+            </action>
             <action dev="henrib" type="fix" issue="JEXL-315" due-to="Mike Bartlett">
                JxltEngine literal string strings ending in \ $ or # throw JxltEngine$Exception
             </action>
diff --git a/src/test/java/org/apache/commons/jexl3/ScriptCallableTest.java b/src/test/java/org/apache/commons/jexl3/ScriptCallableTest.java
index 313562d..63b5b0d 100644
--- a/src/test/java/org/apache/commons/jexl3/ScriptCallableTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ScriptCallableTest.java
@@ -27,6 +27,7 @@ import java.util.concurrent.FutureTask;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
 import org.apache.commons.jexl3.internal.Script;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -67,7 +68,6 @@ public class ScriptCallableTest extends JexlTestCase {
 
     @Test
     public void testCallableCancel() throws Exception {
-        List<Runnable> lr = null;
         final Semaphore latch = new Semaphore(0);
         JexlContext ctxt = new MapContext();
         ctxt.set("latch", latch);
@@ -85,6 +85,7 @@ public class ScriptCallableTest extends JexlTestCase {
         ExecutorService executor = Executors.newFixedThreadPool(2);
         Future<?> future = executor.submit(c);
         Future<?> kfc = executor.submit(kc);
+        List<Runnable> lr;
         try {
             Assert.assertTrue((Boolean) kfc.get());
             t = future.get();
@@ -98,7 +99,55 @@ public class ScriptCallableTest extends JexlTestCase {
         Assert.assertTrue(c.isCancelled());
         Assert.assertTrue(lr == null || lr.isEmpty());
     }
+    
+    public static class CancellationContext extends MapContext implements JexlContext.CancellationHandle {
+        private final AtomicBoolean cancellation;
+        
+        CancellationContext(AtomicBoolean c) {
+            cancellation = c;
+        }
+        @Override
+        public AtomicBoolean getCancellation() {
+            return cancellation;
+        }
+    }
+    
+    // JEXL-317
+    @Test
+    public void testCallableCancellation() throws Exception {
+        final Semaphore latch = new Semaphore(0);
+        final AtomicBoolean cancel = new AtomicBoolean(false);
+        JexlContext ctxt = new CancellationContext(cancel);
+        ctxt.set("latch", latch);
 
+        JexlScript e = JEXL.createScript("latch.release(); while(true);");
+        final Script.Callable c = (Script.Callable) e.callable(ctxt);
+        Object t = 42;
+        Callable<Object> kc = new Callable<Object>() {
+            @Override
+            public Object call() throws Exception {
+                latch.acquire();
+                return cancel.compareAndSet(false, true);
+            }
+        };
+        ExecutorService executor = Executors.newFixedThreadPool(2);
+        Future<?> future = executor.submit(c);
+        Future<?> kfc = executor.submit(kc);
+        List<Runnable> lr;
+        try {
+            Assert.assertTrue((Boolean) kfc.get());
+            t = future.get();
+            Assert.fail("should have been cancelled");
+        } catch (ExecutionException xexec) {
+            // ok, ignore
+            Assert.assertTrue(xexec.getCause() instanceof JexlException.Cancel);
+        } finally {
+            lr = executor.shutdownNow();
+        }
+        Assert.assertTrue(c.isCancelled());
+        Assert.assertTrue(lr == null || lr.isEmpty());
+    }
+    
     @Test
     public void testCallableTimeout() throws Exception {
         List<Runnable> lr = null;


[commons-jexl] 02/02: JEXL-307: fixed lexical feature handling of variable scope in for/lambda units Task #JEXL-307 - Variable redeclaration option

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git

commit 53e5e77cc9ec42c96840c8f4ae04dc500e0baf63
Author: henrib <he...@apache.org>
AuthorDate: Mon Dec 2 17:09:50 2019 +0100

    JEXL-307: fixed lexical feature handling of variable scope in for/lambda units
    Task #JEXL-307 - Variable redeclaration option
---
 .../commons/jexl3/parser/ASTForeachStatement.java  | 62 ----------------------
 .../apache/commons/jexl3/parser/JexlParser.java    | 44 +++++++--------
 .../org/apache/commons/jexl3/parser/Parser.jjt     | 15 +++---
 .../java/org/apache/commons/jexl3/LexicalTest.java | 14 +++++
 4 files changed, 42 insertions(+), 93 deletions(-)

diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java b/src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java
deleted file mode 100644
index 1ad61c0..0000000
--- a/src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.commons.jexl3.parser;
-
-import org.apache.commons.jexl3.internal.LexicalScope;
-
-/**
- * Declares a local variable.
- */
-public class ASTForeachStatement extends JexlNode implements JexlParser.LexicalUnit {
-    private LexicalScope locals = null;
-    
-    public ASTForeachStatement(int id) {
-        super(id);
-    }
-
-    public ASTForeachStatement(Parser p, int id) {
-        super(p, id);
-    }
-
-    @Override
-    public Object jjtAccept(ParserVisitor visitor, Object data) {
-        return visitor.visit(this, data);
-    }
-    
-    @Override
-    public boolean declareSymbol(int symbol) {
-        if (locals == null) {
-            locals  = new LexicalScope(null);
-        }
-        return locals.declareSymbol(symbol);
-    }
-    
-    @Override
-    public int getSymbolCount() {
-        return locals == null? 0 : locals.getSymbolCount();
-    }
-
-    @Override
-    public boolean hasSymbol(int symbol) {
-        return locals == null? false : locals.hasSymbol(symbol);
-    }    
-    
-    @Override
-    public void clearUnit() {
-        locals = null;
-    }
-}
\ No newline at end of file
diff --git a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
index 4aa2c05..d32feb5 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
@@ -61,7 +61,7 @@ public abstract class JexlParser extends StringParser {
     /**
      * When parsing inner functions/lambda, need to stack the scope (sic).
      */
-    protected Deque<Scope> frames = new ArrayDeque<Scope>();
+    protected final Deque<Scope> frames = new ArrayDeque<Scope>();
     /**
      * The list of pragma declarations.
      */
@@ -73,7 +73,7 @@ public abstract class JexlParser extends StringParser {
     /**
      * Stack of parsing loop counts.
      */
-    protected Deque<Integer> loopCounts = new ArrayDeque<Integer>();
+    protected final Deque<Integer> loopCounts = new ArrayDeque<Integer>();
     /**
      * Lexical unit merge, next block push is swallowed.
      */
@@ -85,7 +85,7 @@ public abstract class JexlParser extends StringParser {
     /**
      * Stack of lexical blocks.
      */
-    protected Deque<LexicalUnit> blocks = new ArrayDeque<LexicalUnit>();
+    protected final Deque<LexicalUnit> blocks = new ArrayDeque<LexicalUnit>();
 
     /**
      * A lexical unit is the container defining local symbols and their
@@ -242,19 +242,9 @@ public abstract class JexlParser extends StringParser {
 
     /**
      * Pushes a new lexical unit.
-     * <p>The merge flag allows the for(...) and lamba(...) constructs to
-     * merge in the next block since their loop-variable/parameter spill in the
-     * same lexical unit as their first block.
      * @param unit the new lexical unit
-     * @param merge whether the next unit merges in this one
-     */
-    protected void pushUnit(LexicalUnit unit, boolean merge) {
-        if (merge) {
-            mergeBlock = true;
-        } else if (mergeBlock) {
-            mergeBlock = false;
-            return;
-        }
+     */
+    protected void pushUnit(LexicalUnit unit) {
         if (block != null) {
             blocks.push(block);
         }
@@ -262,14 +252,6 @@ public abstract class JexlParser extends StringParser {
     }
 
     /**
-     * Pushes a block as new lexical unit.
-     * @param unit the lexical unit
-     */
-    protected void pushUnit(LexicalUnit unit) {
-        pushUnit(unit, false);
-    }
-
-    /**
      * Restores the previous lexical unit.
      * @param unit restores the previous lexical scope
      */
@@ -295,8 +277,20 @@ public abstract class JexlParser extends StringParser {
             Integer symbol = frame.getSymbol(name);
             if (symbol != null) {
                 // can not reuse a local as a global
-                if (!block.hasSymbol(symbol) && getFeatures().isLexical()) {
-                    throw new JexlException(identifier,  name + ": variable is not defined");
+                if (getFeatures().isLexical()) {
+                    // one of the lexical blocks above must declare it
+                    if (!block.hasSymbol(symbol)) {
+                        boolean declared = false;
+                        for (LexicalUnit u : blocks) {
+                            if (u.hasSymbol(symbol)) {
+                                declared = true;
+                                break;
+                            }
+                        }
+                        if (!declared) {
+                            throw new JexlException(identifier, name + ": variable is not defined");
+                        }
+                    }
                 }
                 identifier.setSymbol(symbol, name);
             }
diff --git a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
index cdb19ca..a750261 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
+++ b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
@@ -296,7 +296,7 @@ ASTJexlScript JexlScript(Scope frame) : {
 }
 {
    {
-        pushUnit(jjtThis, frame != null && frame.getArgCount() > 0);
+        pushUnit(jjtThis);
    }
         ( ( Statement() )*) <EOF>
    {
@@ -310,7 +310,7 @@ ASTJexlScript JexlExpression(Scope frame) #JexlScript : {
 }
 {   
    {
-        pushUnit(jjtThis, true);
+        pushUnit(jjtThis);
    }
    ( Expression() )? <EOF>
    {
@@ -399,7 +399,10 @@ void Break() #Break : {
 
 void ForeachStatement() : {}
 {
-    { pushUnit(jjtThis, true); } <FOR> <LPAREN> ForEachVar() <COLON>  Expression() <RPAREN> { loopCount += 1; } (LOOKAHEAD(1) Block() | Statement()) { loopCount -= 1; popUnit(jjtThis); }
+    <FOR> <LPAREN> ForEachVar() <COLON>  Expression() <RPAREN>
+    { loopCount += 1; }
+        (LOOKAHEAD(1) Block() | Statement())
+    { loopCount -= 1; }
 }
 
 void ForEachVar() #Reference : {}
@@ -834,11 +837,11 @@ void Lambda() #JexlLambda() :
    pushFrame();
 }
 {
-  { pushUnit(jjtThis, true); } <FUNCTION> Parameters() Block() { popUnit(jjtThis); }
+  { pushUnit(jjtThis); } <FUNCTION> Parameters() Block() { popUnit(jjtThis); }
   |
-  { pushUnit(jjtThis, true); } Parameters() <LAMBDA> Block() { popUnit(jjtThis); }
+  { pushUnit(jjtThis); } Parameters() <LAMBDA> Block() { popUnit(jjtThis); }
   |
-  { pushUnit(jjtThis, true); } Parameter() <LAMBDA> Block() { popUnit(jjtThis); }
+  { pushUnit(jjtThis); } Parameter() <LAMBDA> Block() { popUnit(jjtThis); }
 }
 
 
diff --git a/src/test/java/org/apache/commons/jexl3/LexicalTest.java b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
index 8a6273f..64cfe6b 100644
--- a/src/test/java/org/apache/commons/jexl3/LexicalTest.java
+++ b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
@@ -480,4 +480,18 @@ public class LexicalTest {
         Object result = s42.execute(jc);
         Assert.assertEquals(42, result);
     }
+        
+    @Test
+    public void testInnerAccess0() throws Exception {
+        JexlFeatures f = new JexlFeatures();
+        f.lexical(true);
+        JexlEngine jexl = new JexlBuilder().strict(true).features(f).create();
+        JexlScript script = jexl.createScript("var x = 32; (()->{ for(var x : null) { var c = 0; {return x; }} })();");
+    }
+    
+    @Test
+    public void testInnerAccess1() throws Exception {
+        JexlEngine jexl = new JexlBuilder().strict(true).lexical(true).create();
+        JexlScript script = jexl.createScript("var x = 32; (()->{ for(var x : null) { var c = 0; {return x; }} })();");
+    }
 }