From df9233ed852f987ac4d257fd341791be9cb340a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9D=D0=B8=D0=BA=D0=BE=D0=BB=D0=B0=D0=B9=20=D0=91=D0=B5?= =?UTF-8?q?=D0=BB=D0=BE=D0=BD=D0=BE=D0=B3=D0=BE=D0=B2?= Date: Fri, 23 Oct 2026 11:35:54 +0300 Subject: [PATCH] fix(core): inconsistency connection state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the current version of the code, if a timeout value less than or equal to 0 is passed, an exception is thrown, but the connection remains stuck in the CONNECTING state. Subsequent calls to the Connection#connect(...) method on the same object return null instead of a proper CompletableFuture. In other words, failures or errors during the execution of Connection#connect(...) should not leave the Connection object in an inconsistent state. Subsequent calls should be handled correctly; the method must return a CompletableFuture—either one that completes successfully or one that contains an error, as per the contract of the CompletionStage interface. Providing not positive timeout value throws an exception. --- CHANGELOG.md | 6 +++++ .../core/connection/ConnectionImpl.java | 7 ++--- .../ConnectionToTarantoolTest.java | 26 +++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) 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));