diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c7f628a..23db58fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ - Fixed a bug: the timeout in IProtoBegin was being specified in milliseconds (as a `long`), but the [protocol](https://www.tarantool.io/ru/doc/latest/reference/internals/iproto/streams/#iproto-begin) requires the timeout to be passed in seconds (as a `double`). +- Fixed a bug where the `Connection#connect()` method would leave the connection in a `CONNECTING` + state if a failure occurred before the channel was created (for example, when an + invalid—non-positive—timeout value was provided). As a result, all subsequent calls to this + method would return `null` instead of a `CompletableFuture`. The method now always returns a + `CompletableFuture` object, which either completes successfully (upon receipt of a greeting) or + terminates with an error. Providing not positive timeout value throws an exception. ### Documentation diff --git a/tarantool-core/src/main/java/io/tarantool/core/connection/ConnectionImpl.java b/tarantool-core/src/main/java/io/tarantool/core/connection/ConnectionImpl.java index 892806c4..48825a2a 100644 --- a/tarantool-core/src/main/java/io/tarantool/core/connection/ConnectionImpl.java +++ b/tarantool-core/src/main/java/io/tarantool/core/connection/ConnectionImpl.java @@ -181,6 +181,10 @@ public Connection listen(Consumer listener) { @Override public synchronized CompletableFuture connect(InetSocketAddress address, long timeoutMs) throws IllegalStateException { + if (timeoutMs <= 0) { + throw new IllegalArgumentException("Timeout must be greater than zero!"); + } + if (state.compareAndSet(State.CLOSED, State.CONNECTING)) { closedBy.set(CloseReason.NO_REASON); log.debug("{} is connecting to {}", this, address); @@ -407,9 +411,6 @@ private void fire(ConnectionCloseEvent event, Throwable ex) { private void scheduleGreetingTimeout(CompletableFuture promise, long timeoutMs) { log.debug("Schedule timeout for connect in milliseconds: {}", timeoutMs); - if (timeoutMs <= 0) { - throw new IllegalStateException("Timeout must be greater than zero!"); - } timeoutHandler = this.timerService.newTimeout( timeoutHandler -> { diff --git a/tarantool-core/src/test/java/io/tarantool/core/integration/ConnectionToTarantoolTest.java b/tarantool-core/src/test/java/io/tarantool/core/integration/ConnectionToTarantoolTest.java index 2262cfbb..f3aa6f1e 100644 --- a/tarantool-core/src/test/java/io/tarantool/core/integration/ConnectionToTarantoolTest.java +++ b/tarantool-core/src/test/java/io/tarantool/core/integration/ConnectionToTarantoolTest.java @@ -34,6 +34,7 @@ import static org.msgpack.value.ValueFactory.newMap; import io.netty.channel.ConnectTimeoutException; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; @@ -390,6 +391,31 @@ public void testConnectAndSendByMultipleThreads() throws InterruptedException { assertEquals(CONCURRENT_THREADS_COUNT, success); } + /** + * This test checks for an inconsistent state caused by passing a non-positive timeout during a + * connection attempt, followed by a subsequent attempt to connect. + */ + @Test + void testConnectWithNonPositiveTimeoutForFirstAttempt() throws Exception { + try (Connection connection = factory.create()) { + InetSocketAddress address = tt.mappedAddress(); + + Assertions.assertThrows( + IllegalArgumentException.class, () -> connection.connect(address, -1L)); + + final CompletableFuture connectionFuture = connection.connect(address, 1_000L); + Assertions.assertNotNull(connectionFuture, "Connection future must be not null!"); + + final Greeting greeting = connectionFuture.join(); + Assertions.assertNotNull(greeting, "Greeting must be not null!"); + Assertions.assertTrue(connection.isConnected(), "Connection must be connected"); + + final Greeting secondGreeting = connection.connect(address, 5_000L).join(); + + Assertions.assertSame(greeting, secondGreeting, "Must be same!"); + } + } + private IProtoRequest createSelectRequest(int syncId) { Map rawHeader = new HashMap<>(); rawHeader.put(newInteger(IPROTO_REQUEST_TYPE), newInteger(IPROTO_SELECT));