From 8883e866658847a6422f02b0b74eb3bf313508ab Mon Sep 17 00:00:00 2001 From: Victor Mosin Date: Mon, 8 Apr 2024 15:02:45 +0200 Subject: [PATCH 1/3] feat: made sure feel-interpreter's evaluation method can be interrupted --- .../camunda/feel/impl/interpreter/FeelInterpreter.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala b/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala index ad7064f76..53dfda033 100644 --- a/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala +++ b/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala @@ -39,7 +39,11 @@ import java.time.{Duration, Period} */ class FeelInterpreter { - def eval(expression: Exp)(implicit context: EvalContext): Val = + def eval(expression: Exp)(implicit context: EvalContext): Val = { + if (Thread.interrupted()) { + throw new InterruptedException() + } + expression match { // literals @@ -269,6 +273,7 @@ class FeelInterpreter { case exp => error(EvaluationFailureType.UNKNOWN, s"Unsupported expression '$exp'") } + } private def mapEither[T, R]( it: Iterable[T], From d286a1312ee3c5305cd0fd3bb04d9f8fdd7c4347 Mon Sep 17 00:00:00 2001 From: Victor Mosin Date: Wed, 10 Apr 2024 12:41:24 +0200 Subject: [PATCH 2/3] test: added test --- .../impl/interpreter/FeelInterpreter.scala | 1 + .../InterpreterInterruptionTest.scala | 86 +++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 src/test/scala/org/camunda/feel/impl/interpreter/InterpreterInterruptionTest.scala diff --git a/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala b/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala index 53dfda033..54f4b14fe 100644 --- a/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala +++ b/src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala @@ -40,6 +40,7 @@ import java.time.{Duration, Period} class FeelInterpreter { def eval(expression: Exp)(implicit context: EvalContext): Val = { + // Check if the current thread was interrupted, otherwise long-running evaluations can not be interrupted and fully block the thread if (Thread.interrupted()) { throw new InterruptedException() } diff --git a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterInterruptionTest.scala b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterInterruptionTest.scala new file mode 100644 index 000000000..069330359 --- /dev/null +++ b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterInterruptionTest.scala @@ -0,0 +1,86 @@ +/* + * Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH + * under one or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information regarding copyright + * ownership. Camunda licenses this file to you under the Apache License, + * Version 2.0; 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.camunda.feel.impl.interpreter + +import org.camunda.feel.impl.{EvaluationResultMatchers, FeelEngineTest} +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.lang.management.{ManagementFactory, ThreadInfo} +import java.util.concurrent.{Executors} +import concurrent.{ExecutionContext} + +/** @author + * Victor Mosin + */ +class InterpreterInterruptionTest + extends AnyFlatSpec + with Matchers + with FeelEngineTest + with EvaluationResultMatchers { + + protected implicit val context = + ExecutionContext.fromExecutorService(Executors.newSingleThreadExecutor()) + + "A long-running evaluation invocation" should "be interrupted after X time" in { + val countDownLatch = new java.util.concurrent.CountDownLatch(1) + val thread = new Thread { + override def run { + try { + evaluateExpression( + expression = "count(for x in 1..(2 ** 16) return {\"power\": 2 ** x}) > 0" + ) + } catch { + case _: InterruptedException => + countDownLatch.countDown() + } + } + } + + thread.start() + Thread.sleep(1000) // Let evaluation start + thread.interrupt() + countDownLatch.await() + + assert(!threadDump.contains("FeelInterpreter")) + } + + /** Dumps all threads to a string Adapted from + * https://crunchify.com/how-to-generate-java-thread-dump-programmatically/ + */ + private def threadDump: String = { + val dump = new StringBuilder + val threadMXBean = ManagementFactory.getThreadMXBean + val threadInfos: Array[ThreadInfo] = + threadMXBean.getThreadInfo(threadMXBean.getAllThreadIds, 100) + for (threadInfo <- threadInfos) { + dump.append('"') + dump.append(threadInfo.getThreadName) + dump.append("\" ") + val state: Thread.State = threadInfo.getThreadState + dump.append("\n java.lang.Thread.State: ") + dump.append(state) + val stackTraceElements: Array[StackTraceElement] = threadInfo.getStackTrace + for (stackTraceElement <- stackTraceElements) { + dump.append("\n at ") + dump.append(stackTraceElement) + } + dump.append("\n\n") + } + dump.toString + } +} From 41131fe3de083c89989a41fca1eddb1e16d12bea Mon Sep 17 00:00:00 2001 From: Victor Mosin Date: Thu, 18 Apr 2024 10:13:33 +0200 Subject: [PATCH 3/3] test: minor review changes --- .../InterpreterInterruptionTest.scala | 44 +++---------------- 1 file changed, 7 insertions(+), 37 deletions(-) diff --git a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterInterruptionTest.scala b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterInterruptionTest.scala index 069330359..946afdf06 100644 --- a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterInterruptionTest.scala +++ b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterInterruptionTest.scala @@ -16,22 +16,17 @@ */ package org.camunda.feel.impl.interpreter -import org.camunda.feel.impl.{EvaluationResultMatchers, FeelEngineTest} +import org.camunda.feel.impl.FeelEngineTest import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -import java.lang.management.{ManagementFactory, ThreadInfo} -import java.util.concurrent.{Executors} -import concurrent.{ExecutionContext} +import java.util.concurrent.{Executors, TimeUnit} +import concurrent.ExecutionContext /** @author * Victor Mosin */ -class InterpreterInterruptionTest - extends AnyFlatSpec - with Matchers - with FeelEngineTest - with EvaluationResultMatchers { +class InterpreterInterruptionTest extends AnyFlatSpec with Matchers with FeelEngineTest { protected implicit val context = ExecutionContext.fromExecutorService(Executors.newSingleThreadExecutor()) @@ -52,35 +47,10 @@ class InterpreterInterruptionTest } thread.start() - Thread.sleep(1000) // Let evaluation start + Thread.sleep(100) // Let evaluation start thread.interrupt() - countDownLatch.await() + val wasInterrupted = countDownLatch.await(1, TimeUnit.SECONDS) - assert(!threadDump.contains("FeelInterpreter")) - } - - /** Dumps all threads to a string Adapted from - * https://crunchify.com/how-to-generate-java-thread-dump-programmatically/ - */ - private def threadDump: String = { - val dump = new StringBuilder - val threadMXBean = ManagementFactory.getThreadMXBean - val threadInfos: Array[ThreadInfo] = - threadMXBean.getThreadInfo(threadMXBean.getAllThreadIds, 100) - for (threadInfo <- threadInfos) { - dump.append('"') - dump.append(threadInfo.getThreadName) - dump.append("\" ") - val state: Thread.State = threadInfo.getThreadState - dump.append("\n java.lang.Thread.State: ") - dump.append(state) - val stackTraceElements: Array[StackTraceElement] = threadInfo.getStackTrace - for (stackTraceElement <- stackTraceElements) { - dump.append("\n at ") - dump.append(stackTraceElement) - } - dump.append("\n\n") - } - dump.toString + wasInterrupted should be(true) } }