You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by xi...@apache.org on 2023/02/24 17:56:47 UTC
[tinkerpop] branch 3.6-dev updated: Fix server not handling StackOverflowError in bytecode traversals (#1979)
This is an automated email from the ASF dual-hosted git repository.
xiazcy pushed a commit to branch 3.6-dev
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/3.6-dev by this push:
new d1479f4706 Fix server not handling StackOverflowError in bytecode traversals (#1979)
d1479f4706 is described below
commit d1479f4706ec243e14e780429876639db67edc5e
Author: Cole Greer <11...@users.noreply.github.com>
AuthorDate: Fri Feb 24 09:56:39 2023 -0800
Fix server not handling StackOverflowError in bytecode traversals (#1979)
Fixes TINKERPOP-2767.
The existing error handling in TraversalOpProcessor and SessionOpProcessor during bytecode iteration was
catching all exceptions sending an appropriate error response to the client. The OpProcessor however was
not catching a StackOverflowError which could be induced by running a query which contains a
large repeat step. This Error was being caught by FutureTask.run() but GremlinServer never wait's
on this task or checks the results which caused this Error to be lost.
A similar issue was found to exist in AbstractSession.process().
This commit adjusts the existing error handling code to catch any Throwable during bytecode
iteration so clients will receive error messages and codes for server errors as well as exceptions.
Any errors are re-thrown such that the evalFuture FutureTask will continue to have an exception set
correctly (although GremlinServer currently does not use this for anything).
---
.../gremlin/server/handler/AbstractSession.java | 4 +--
.../server/op/session/SessionOpProcessor.java | 32 ++++++++++++++--------
.../server/op/traversal/TraversalOpProcessor.java | 16 +++++++----
3 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/AbstractSession.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/AbstractSession.java
index 0b22b113af..d891e50a17 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/AbstractSession.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/AbstractSession.java
@@ -281,8 +281,8 @@ public abstract class AbstractSession implements Session, AutoCloseable {
if (itty.isPresent())
handleIterator(sessionTask, itty.get());
- } catch (Exception ex) {
- handleException(sessionTask, ex);
+ } catch (Throwable t) {
+ handleException(sessionTask, t);
} finally {
timer.stop();
}
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
index ff00bc6f57..039d1ca57a 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
@@ -451,10 +451,11 @@ public class SessionOpProcessor extends AbstractEvalOpProcessor {
}
onError(graph, context);
}
- } catch (Exception ex) {
+ } catch (Throwable t) {
+ onError(graph, context);
// if any exception in the chain is TemporaryException or Failure then we should respond with the
// right error code so that the client knows to retry
- final Optional<Throwable> possibleSpecialException = determineIfSpecialException(ex);
+ final Optional<Throwable> possibleSpecialException = determineIfSpecialException(t);
if (possibleSpecialException.isPresent()) {
final Throwable special = possibleSpecialException.get();
final ResponseMessage.Builder specialResponseMsg = ResponseMessage.build(msg).
@@ -469,12 +470,15 @@ public class SessionOpProcessor extends AbstractEvalOpProcessor {
}
context.writeAndFlush(specialResponseMsg.create());
} else {
- logger.warn(String.format("Exception processing a Traversal on request [%s].", msg.getRequestId()), ex);
+ logger.warn(String.format("Exception processing a Traversal on request [%s].", msg.getRequestId()), t);
context.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR)
- .statusMessage(ex.getMessage())
- .statusAttributeException(ex).create());
+ .statusMessage(t.getMessage())
+ .statusAttributeException(t).create());
+ }
+ if (t instanceof Error) {
+ //Re-throw any errors to be handled by and set as the result of evalFuture
+ throw t;
}
- onError(graph, context);
} finally {
// todo: timer matter???
//timerContext.stop();
@@ -529,10 +533,11 @@ public class SessionOpProcessor extends AbstractEvalOpProcessor {
.statusAttributes(attributes)
.create());
- } catch (Exception ex) {
+ } catch (Throwable t) {
+ onError(graph, context);
// if any exception in the chain is TemporaryException or Failure then we should respond with the
// right error code so that the client knows to retry
- final Optional<Throwable> possibleSpecialException = determineIfSpecialException(ex);
+ final Optional<Throwable> possibleSpecialException = determineIfSpecialException(t);
if (possibleSpecialException.isPresent()) {
final Throwable special = possibleSpecialException.get();
final ResponseMessage.Builder specialResponseMsg = ResponseMessage.build(msg).
@@ -548,12 +553,15 @@ public class SessionOpProcessor extends AbstractEvalOpProcessor {
context.writeAndFlush(specialResponseMsg.create());
} else {
logger.warn(String.format("Exception processing a Traversal on request [%s] to %s the transaction.",
- msg.getRequestId(), commit ? "commit" : "rollback"), ex);
+ msg.getRequestId(), commit ? "commit" : "rollback"), t);
context.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR)
- .statusMessage(ex.getMessage())
- .statusAttributeException(ex).create());
+ .statusMessage(t.getMessage())
+ .statusAttributeException(t).create());
+ }
+ if (t instanceof Error) {
+ //Re-throw any errors to be handled by and set as the result the FutureTask
+ throw t;
}
- onError(graph, context);
}
return null;
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
index 5ddaf693ed..1136046d6b 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
@@ -256,10 +256,11 @@ public class TraversalOpProcessor extends AbstractOpProcessor {
}
onError(graph, context, ex);
}
- } catch (Exception ex) {
+ } catch (Throwable t) {
+ onError(graph, context, t);
// if any exception in the chain is TemporaryException or Failure then we should respond with the
// right error code so that the client knows to retry
- final Optional<Throwable> possibleSpecialException = determineIfSpecialException(ex);
+ final Optional<Throwable> possibleSpecialException = determineIfSpecialException(t);
if (possibleSpecialException.isPresent()) {
final Throwable special = possibleSpecialException.get();
final ResponseMessage.Builder specialResponseMsg = ResponseMessage.build(msg).
@@ -274,12 +275,15 @@ public class TraversalOpProcessor extends AbstractOpProcessor {
}
context.writeAndFlush(specialResponseMsg.create());
} else {
- logger.warn(String.format("Exception processing a Traversal on request [%s].", msg.getRequestId()), ex);
+ logger.warn(String.format("Exception processing a Traversal on request [%s].", msg.getRequestId()), t);
context.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR)
- .statusMessage(ex.getMessage())
- .statusAttributeException(ex).create());
+ .statusMessage(t.getMessage())
+ .statusAttributeException(t).create());
+ if (t instanceof Error) {
+ //Re-throw any errors to be handled by and set as the result of evalFuture
+ throw t;
+ }
}
- onError(graph, context, ex);
} finally {
timerContext.stop();
}