diff --git a/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java b/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java index dd78bd421..76becad77 100644 --- a/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java +++ b/src/main/java/org/jboss/ejb/client/EJBClientInvocationContext.java @@ -71,7 +71,7 @@ public final class EJBClientInvocationContext extends AbstractInvocationContext private final EJBReceiverInvocationContext receiverInvocationContext = new EJBReceiverInvocationContext(this); private final EJBClientContext.InterceptorList interceptorList; private final long startTime = System.nanoTime(); - private final long timeout; + private long timeout; // Invocation state private final Object lock = new Object(); @@ -344,7 +344,7 @@ void sendRequestInitial() { // back to the start of the chain; decide what to do next. synchronized (lock) { try { - assert state == State.SENT; + assert state == State.SENT || state == State.READY; // from here we can go to: READY, or WAITING, or retry SENDING. Supplier pendingFailure = this.pendingFailure; EJBReceiverInvocationContext.ResultProducer resultProducer = this.resultProducer; @@ -392,7 +392,7 @@ void sendRequestInitial() { // didn't make it to the end of the chain even... but we won't suppress the thrown exception transition(State.SENT); } - assert state == State.SENT; + assert state == State.SENT || state == State.READY; try { // from here we can go to: FAILED, READY, or retry SENDING. Supplier pendingFailure = this.pendingFailure; @@ -525,7 +525,7 @@ public void sendRequest() throws Exception { } synchronized (lock) { try { - if (state != State.SENT) { + if (state != State.SENT && state != State.READY) { assert state == State.SENDING; transition(State.SENT); throw Logs.INVOCATION.requestNotSent(); @@ -702,13 +702,19 @@ void resultReady(EJBReceiverInvocationContext.ResultProducer resultProducer) { synchronized (lock) { if (state.isWaiting() && this.resultProducer == null) { this.resultProducer = resultProducer; - if (state == State.WAITING) { + if (state == State.WAITING || state == State.SENT) { transition(State.READY); } checkStateInvariants(); + if (log.isTraceEnabled()) { + log.tracef("Result is ready for %s: result producer: %s, current state: %s", this, resultProducer, state); + } return; } checkStateInvariants(); + if (log.isTraceEnabled()) { + log.tracef("Result discarded for %s: result producer: %s, current state: %s", this, resultProducer, state); + } } // for whatever reason, we don't care resultProducer.discardResult(); @@ -799,6 +805,9 @@ private void transition(State newState) { final Object lock = this.lock; Assert.assertHoldsLock(lock); final State oldState = this.state; + if (oldState == newState) { + return; + } if (log.isTraceEnabled()) { StackTraceElement caller = (new Exception()).getStackTrace()[1]; log.tracef("Transitioning %s from %s to %s (%s)", this, oldState, newState, caller); @@ -821,7 +830,7 @@ private void transition(State newState) { break; } case CONSUMING: { - assert newState == State.SENDING || newState == State.DONE; + assert newState == State.SENDING || newState == State.DONE || newState == State.READY; break; } default: { @@ -931,7 +940,7 @@ Object awaitResponse() throws Exception { case SENT: case CONSUMING: case WAITING: { - if (timeout <= 0) { + if (timeout <= 0 || timedOut) { // no timeout; lighter code path try { checkStateInvariants(); @@ -950,7 +959,13 @@ Object awaitResponse() throws Exception { if (remaining == 0L) { // timed out timedOut = true; - resultReady(new ThrowableResult(() -> new TimeoutException("No invocation response received in " + timeout + " milliseconds"))); + 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"))); + } } else try { checkStateInvariants(); try { diff --git a/src/test/java/org/jboss/ejb/client/test/byteman/TimeoutRetryTestCase.java b/src/test/java/org/jboss/ejb/client/test/byteman/TimeoutRetryTestCase.java new file mode 100644 index 000000000..d1f3d5a4d --- /dev/null +++ b/src/test/java/org/jboss/ejb/client/test/byteman/TimeoutRetryTestCase.java @@ -0,0 +1,168 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2017 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed 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.jboss.ejb.client.test.byteman; + +import org.jboss.byteman.contrib.bmunit.BMScript; +import org.jboss.byteman.contrib.bmunit.BMUnitRunner; +import org.jboss.ejb.client.EJBClient; +import org.jboss.ejb.client.StatelessEJBLocator; +import org.jboss.ejb.client.URIAffinity; +import org.jboss.ejb.client.legacy.JBossEJBProperties; +import org.jboss.ejb.client.test.ClassCallback; +import org.jboss.ejb.client.test.common.DummyServer; +import org.jboss.ejb.client.test.common.Echo; +import org.jboss.ejb.client.test.common.EchoBean; +import org.jboss.logging.Logger; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import javax.ejb.EJBException; +import javax.ejb.NoSuchEJBException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +/** + * Tests that a hang does not happen if timeout happens concurrently with retry + */ +@RunWith(BMUnitRunner.class) +@BMScript(dir = "src/test/resources") +public class TimeoutRetryTestCase { + + private static final Logger logger = Logger.getLogger(TimeoutRetryTestCase.class); + private static final String PROPERTIES_FILE = "jboss-ejb-client.properties"; + + private DummyServer server; + private boolean serverStarted = false; + + // module + private static final String APP_NAME = "my-foo-app"; + private static final String MODULE_NAME = "my-bar-module"; + private static final String DISTINCT_NAME = ""; + + private static final String SERVER_NAME = "test-server"; + + + /** + * Do any general setup here + * + * @throws Exception + */ + @BeforeClass + public static void beforeClass() throws Exception { + // trigger the static init of the correct proeprties file - this also depends on running in forkMode=always + JBossEJBProperties ejbProperties = JBossEJBProperties.fromClassPath(TimeoutRetryTestCase.class.getClassLoader(), PROPERTIES_FILE); + JBossEJBProperties.getContextManager().setGlobalDefault(ejbProperties); + + // Launch callback if needed + ClassCallback.beforeClassCallback(); + } + + /** + * Do any test specific setup here + */ + @Before + public void beforeTest() throws Exception { + // start a server + server = new DummyServer("localhost", 6999, SERVER_NAME); + server.start(); + serverStarted = true; + logger.info("Started server ..."); + + server.register(APP_NAME, MODULE_NAME, DISTINCT_NAME, "missing", new EchoBean()); + logger.info("Registered module ..."); + } + + /** + * Test a basic invocation + */ + @Test + public void testInvocationWithURIAffinity() { + logger.info("Testing invocation on proxy with URIAffinity"); + + // create a proxy for invocation + final StatelessEJBLocator statelessEJBLocator = new StatelessEJBLocator(Echo.class, APP_NAME, MODULE_NAME, Echo.class.getSimpleName(), DISTINCT_NAME); + final Echo proxy = EJBClient.createProxy(statelessEJBLocator); + EJBClient.setInvocationTimeout(proxy, 1, TimeUnit.SECONDS); + URI uri = null; + try { + uri = new URI("remote", null, "localhost", 6999, null, null, null); + } catch (URISyntaxException use) { + // + } + EJBClient.setStrongAffinity(proxy, URIAffinity.forUri(uri)); + Assert.assertNotNull("Received a null proxy", proxy); + logger.info("Created proxy for Echo: " + proxy.toString()); + + logger.info("Invoking on proxy..."); + final String message = "hello!"; + long start = System.currentTimeMillis(); + try { + proxy.echo(message); + Assert.fail("Invocation expected to fail"); + } catch (NoSuchEJBException expected) { + boolean found = false; + for (Throwable i : expected.getSuppressed()) { + if (i instanceof TimeoutException) { + found = true; + break; + } + } + if (!found) { + Assert.fail("Expected a supressed timeout exception"); + } + expected.printStackTrace(); + } + //we have a 3s sleep in the retry code + //and a 1s timeout + //so we verify it was less than 1s + Assert.assertTrue("Invocation should have timed out after 1s", System.currentTimeMillis() - start < 2000); + } + + /** + * Do any test-specific tear down here. + */ + @After + public void afterTest() { + server.unregister(APP_NAME, MODULE_NAME, DISTINCT_NAME, Echo.class.getName()); + logger.info("Unregistered module ..."); + + if (serverStarted) { + try { + this.server.stop(); + } catch (Throwable t) { + logger.info("Could not stop server", t); + } + } + logger.info("Stopped server ..."); + } + + /** + * Do any general tear down here. + */ + @AfterClass + public static void afterClass() { + } + +} diff --git a/src/test/resources/org/jboss/ejb/client/test/byteman/TimeoutRetryTestCase.btm b/src/test/resources/org/jboss/ejb/client/test/byteman/TimeoutRetryTestCase.btm new file mode 100644 index 000000000..4d851f41d --- /dev/null +++ b/src/test/resources/org/jboss/ejb/client/test/byteman/TimeoutRetryTestCase.btm @@ -0,0 +1,25 @@ +# +# JBoss, Home of Professional Open Source. +# Copyright 2019 Red Hat, Inc., and individual contributors +# as indicated by the @author tags. +# +# Licensed 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. +# +RULE @1 delay innvocation +CLASS org.jboss.ejb.client.EJBClientInvocationContext +AT ENTRY +METHOD retryOperation +IF TRUE +DO + Thread.sleep(3000) +ENDRULE