From 5d7f42306bca86c88fbb084012dc57e745892844 Mon Sep 17 00:00:00 2001 From: Flavia Rainone Date: Tue, 22 Dec 2020 00:49:28 -0300 Subject: [PATCH 1/3] At EJBClientInvocationContext.resultReady, allow transition from both SENT and CONSUMING to READY, and at awaitResponse, reset timeout to 0 if it is expired. Update asserts accordingly. --- .../org/jboss/ejb/client/EJBClientInvocationContext.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java b/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java index 76becad77..165649965 100644 --- a/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java +++ b/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java @@ -702,7 +702,7 @@ void resultReady(EJBReceiverInvocationContext.ResultProducer resultProducer) { synchronized (lock) { if (state.isWaiting() && this.resultProducer == null) { this.resultProducer = resultProducer; - if (state == State.WAITING || state == State.SENT) { + if (state == State.WAITING || state == State.SENT || state == State.CONSUMING) { transition(State.READY); } checkStateInvariants(); @@ -960,12 +960,7 @@ Object awaitResponse() throws Exception { // timed out timedOut = true; this.timeout = 0; - if (state == State.CONSUMING) { - addSuppressed(new TimeoutException("No invocation response received in " + timeout + " milliseconds")); - transition(State.READY); - } else { - resultReady(new ThrowableResult(() -> new TimeoutException("No invocation response received in " + timeout + " milliseconds"))); - } + resultReady(new ThrowableResult(() -> new TimeoutException("No invocation response received in " + timeout + " milliseconds"))); } else try { checkStateInvariants(); try { From d82bfe241aa4fd6ed1bd42fd832c06a057075944 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Tue, 22 Dec 2020 17:05:50 +1100 Subject: [PATCH 2/3] If state is CONSUMING on timeout then add suppressed. This allows the original failure to be propagated. --- .../org/jboss/ejb/client/EJBClientInvocationContext.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java b/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java index 165649965..76becad77 100644 --- a/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java +++ b/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java @@ -702,7 +702,7 @@ void resultReady(EJBReceiverInvocationContext.ResultProducer resultProducer) { synchronized (lock) { if (state.isWaiting() && this.resultProducer == null) { this.resultProducer = resultProducer; - if (state == State.WAITING || state == State.SENT || state == State.CONSUMING) { + if (state == State.WAITING || state == State.SENT) { transition(State.READY); } checkStateInvariants(); @@ -960,7 +960,12 @@ Object awaitResponse() throws Exception { // timed out timedOut = true; this.timeout = 0; - resultReady(new ThrowableResult(() -> new TimeoutException("No invocation response received in " + timeout + " milliseconds"))); + if (state == State.CONSUMING) { + addSuppressed(new TimeoutException("No invocation response received in " + timeout + " milliseconds")); + transition(State.READY); + } else { + resultReady(new ThrowableResult(() -> new TimeoutException("No invocation response received in " + timeout + " milliseconds"))); + } } else try { checkStateInvariants(); try { From 98375d2c890167bed6382cc6f355f85d54904005 Mon Sep 17 00:00:00 2001 From: Flavia Rainone Date: Wed, 23 Dec 2020 19:16:43 -0300 Subject: [PATCH 3/3] [JBEAP-20756] At EJBServerChannel, do not ommit IOException thrown when writing messages to the client, or else the new message ack timeout configuration in remoting will not be visible in the logs. Also, if an IOException is thrown when writing module availability messages, we must close the channel, or else the client will hang, as the client waits for this message before sending the invocation request to server. --- .../java/org/jboss/ejb/_private/Logs.java | 28 ++++++++++++ .../ejb/protocol/remote/EJBServerChannel.java | 45 ++++++++++--------- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/jboss/ejb/_private/Logs.java b/src/main/java/org/jboss/ejb/_private/Logs.java index 1e76fd050..1ff876b5f 100644 --- a/src/main/java/org/jboss/ejb/_private/Logs.java +++ b/src/main/java/org/jboss/ejb/_private/Logs.java @@ -448,6 +448,34 @@ public interface Logs extends BasicLogger { @Message(id = 516, value = "Exception resolving class %s for unmarshalling; it has either been blacklisted or not whitelisted") InvalidClassException cannotResolveFilteredClass(String clazz); + @LogMessage(level = WARN) + @Message(id = 517, value = "Exception occurred when writing EJB transaction response to invocation %s over channel %s") + void ioExceptionOnTransactionResponseWrite(int invId, Channel channel, @Cause IOException e); + + @LogMessage(level = WARN) + @Message(id = 518, value = "Exception occurred when writing EJB transaction recovery response for invocation %s over channel %s") + void ioExceptionOnTransactionRecoveryResponseWrite(int invId, Channel channel, @Cause IOException e); + + @LogMessage(level = WARN) + @Message(id = 519, value = "Exception occurred when writing EJB response to invocation %s over channel %s") + void ioExceptionOnEJBResponseWrite(int invId, Channel channel, @Cause IOException e); + + @LogMessage(level = WARN) + @Message(id = 520, value = "Exception occurred when writing EJB session open response to invocation %s over channel %s") + void ioExceptionOnEJBSessionOpenResponseWrite(int invId, Channel channel, @Cause IOException e); + + @LogMessage(level = WARN) + @Message(id = 521, value = "Exception occurred when writing proceed async response to invocation %s over channel %s") + void ioExceptionOnProceedAsyncResponseWrite(int invId, Channel channel, @Cause IOException e); + + @LogMessage(level = WARN) + @Message(id = 522, value = "Exception occurred when writing EJB cluster message to channel %s") + void ioExceptionOnEJBClusterMessageWrite(Channel channel, @Cause IOException e); + + @LogMessage(level = WARN) + @Message(id = 523, value = "Exception occurred when writing module availability message, closing channel %s") + void ioExceptionOnModuleAvailabilityWrite(Channel channel, @Cause IOException e); + // Remote messages; no ID for brevity but should be translated @Message(value = "No such EJB: %s") diff --git a/src/main/java/org/jboss/ejb/protocol/remote/EJBServerChannel.java b/src/main/java/org/jboss/ejb/protocol/remote/EJBServerChannel.java index fdea73a71..f6b72a9db 100644 --- a/src/main/java/org/jboss/ejb/protocol/remote/EJBServerChannel.java +++ b/src/main/java/org/jboss/ejb/protocol/remote/EJBServerChannel.java @@ -276,7 +276,7 @@ private void writeTxnResponse(final int invId, final int flag) { PackedInteger.writePackedInteger(os, flag); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB transaction response write failed", e); + Logs.REMOTING.ioExceptionOnTransactionResponseWrite(invId, channel, e); } } @@ -287,7 +287,7 @@ private void writeTxnResponse(final int invId) { os.writeBoolean(false); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB transaction response write failed", e); + Logs.REMOTING.ioExceptionOnTransactionResponseWrite(invId, channel, e); } } @@ -386,7 +386,7 @@ void handleTxnRecoverRequest(final int invId, final MessageInputStream message) marshaller.finish(); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB transaction response write failed", e); + Logs.REMOTING.ioExceptionOnTransactionRecoveryResponseWrite(invId, channel, e); } } @@ -522,8 +522,9 @@ private void writeFailedResponse(final int invId, final Throwable e) { marshaller.writeByte(0); marshaller.finish(); } catch (IOException e2) { + e2.addSuppressed(e); // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB response write failed", e2); + Logs.REMOTING.ioExceptionOnEJBResponseWrite(invId, channel, e2); } } @@ -571,7 +572,7 @@ public void writeNoSuchEJB() { os.writeUTF(message); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB response write failed", e); + Logs.REMOTING.ioExceptionOnEJBResponseWrite(invId, channel, e); } finally { invocations.removeKey(invId); } @@ -595,7 +596,7 @@ public void writeWrongViewType() { } } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB response write failed", e); + Logs.REMOTING.ioExceptionOnEJBResponseWrite(invId, channel, e); } finally { invocations.removeKey(invId); } @@ -607,7 +608,7 @@ public void writeCancelResponse() { os.writeShort(invId); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB response write failed", e); + Logs.REMOTING.ioExceptionOnEJBResponseWrite(invId, channel, e); } finally { invocations.removeKey(invId); } @@ -621,7 +622,7 @@ public void writeNotStateful() { os.writeUTF(message); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB response write failed", e); + Logs.REMOTING.ioExceptionOnEJBResponseWrite(invId, channel, e); } finally { invocations.removeKey(invId); } @@ -657,8 +658,9 @@ protected void writeFailure(Exception reason) { marshaller.writeByte(0); marshaller.finish(); } catch (IOException e) { + e.addSuppressed(reason); // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB response write failed", e); + Logs.REMOTING.ioExceptionOnEJBResponseWrite(invId, channel, e); } finally { invocations.removeKey(invId); } @@ -768,7 +770,7 @@ public void convertToStateful(@NotNull final SessionID sessionId) throws Illegal } } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB session open response write failed", e); + Logs.REMOTING.ioExceptionOnEJBSessionOpenResponseWrite(invId, channel, e); } } } @@ -1009,7 +1011,7 @@ public void writeInvocationResult(final Object result) { os.close(); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB response write failed", e); + Logs.REMOTING.ioExceptionOnEJBResponseWrite(invId, channel, e); } finally { invocations.removeKey(invId); } @@ -1030,7 +1032,7 @@ public void writeProceedAsync() { os.writeShort(invId); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB async response write failed", e); + Logs.REMOTING.ioExceptionOnProceedAsyncResponseWrite(invId, channel, e); } } @@ -1047,7 +1049,7 @@ public void writeNoSuchMethod() { os.writeUTF(message); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB response write failed", e); + Logs.REMOTING.ioExceptionOnEJBResponseWrite(invId, channel, e); } finally { invocations.removeKey(invId); } @@ -1061,7 +1063,7 @@ public void writeSessionNotActive() { os.writeUTF(message); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB response write failed", e); + Logs.REMOTING.ioExceptionOnEJBResponseWrite(invId, channel, e); } finally { invocations.removeKey(invId); } @@ -1086,7 +1088,7 @@ void writeCancellation() { os.writeShort(invId); } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB response write failed", e); + Logs.REMOTING.ioExceptionOnEJBResponseWrite(invId, channel, e); } finally { invocations.removeKey(invId); } else { @@ -1234,7 +1236,7 @@ public void clusterTopology(final List clusterInfoList) { } } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB cluster message write failed", e); + Logs.REMOTING.ioExceptionOnEJBClusterMessageWrite(channel, e); } } @@ -1248,7 +1250,7 @@ public void clusterRemoval(final List clusterNames) { } } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB cluster message write failed", e); + Logs.REMOTING.ioExceptionOnEJBClusterMessageWrite(channel, e); } } @@ -1278,7 +1280,7 @@ public void clusterNewNodesAdded(final ClusterInfo clusterInfo) { } } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB cluster message write failed", e); + Logs.REMOTING.ioExceptionOnEJBClusterMessageWrite(channel, e); } } @@ -1297,7 +1299,7 @@ public void clusterNodesRemoved(final List clusterRemovalInf } } catch (IOException e) { // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB cluster message write failed", e); + Logs.REMOTING.ioExceptionOnEJBClusterMessageWrite(channel, e); } } @@ -1332,8 +1334,11 @@ private void doWrite(final boolean available, final List mo os.writeUTF(distinctName == null ? "" : distinctName); } } catch (IOException e) { + // we need to close connection, as module availability messages occurs when establishing a connection to the server + // prior to sending an invocation request (this will prevent a frozen client at the other side) + channel.closeAsync(); // nothing to do at this point; the client doesn't want the response - Logs.REMOTING.trace("EJB availability message write failed", e); + Logs.REMOTING.ioExceptionOnModuleAvailabilityWrite(channel, e); } } }