ConnectionPool can reuse inactive RedisConnection after channel close, causing "Command still hasn't been written" timeouts #7236

Redisson version

Observed on 3.18.0.

I also checked the RedisExecutor write-timeout branch in later versions (3.26.1, 3.37.0, 3.45.1) and the same writeFuture.cancel(false) behavior appears to still exist.

Problem

We observed repeated errors like:

org.redisson.client.RedisTimeoutException:
Command still hasn't been written into connection!
...
connection: RedisConnection@... [channel=[id: ..., L:/client - R:redis-node], currentCommand=null, usage=1]
...
after 3 retry attempts

Around the same time, Redisson also logged:

Unable to send PING command over channel: [id: ...]
Caused by: org.redisson.client.RedisTimeoutException:
Command execution timeout for command: (PING)

From the source code, PingConnectionHandler calls:

ctx.channel().close();

But this does not set RedisConnection.closed = true. That flag is only set by RedisConnection.closeAsync().

Then ClientConnectionsEntry.releaseConnection() only checks:

if (connection.isClosed()) {
    return;
}
freeConnections.add(connection);
connection.decUsage();

It does not check connection.isActive() / channel.isActive() before returning the connection to the free pool.

Also, ClientConnectionsEntry.pollConnection() returns the connection without filtering inactive master connections:

RedisConnection c = freeConnections.poll();
if (c != null) {
    c.incUsage();
}
return c;

In ConnectionPool.connectTo(), inactive master connections are still accepted:

if (conn != null) {
    if (!conn.isActive() && entry.getNodeType() == NodeType.SLAVE) {
        entry.trySetupFistFail();
    }

    connectedSuccessful(entry, promise, conn);
    return;
}

For master entries, !conn.isActive() does not prevent reuse.

Why this is problematic

If a channel is closed through ctx.channel().close() or otherwise becomes inactive while the RedisConnection.closed flag remains false, the connection can be released back to the pool and borrowed again.

This can produce a loop where Redisson successfully obtains a RedisConnection, but the subsequent write never completes, eventually throwing:

Command still hasn't been written into connection

Minimal unit test

This test demonstrates the pool-level behavior:

@Test
void clientConnectionsEntryCanReuseInactiveChannelIfRedisConnectionIsNotMarkedClosed() throws Exception {
    try (TestConnectionManager manager = new TestConnectionManager()) {
        RedisConnection connection = new RedisConnection(null, new EmbeddedChannel(), new CompletableFuture<>());
        connection.incUsage();

        connection.getChannel().close().syncUninterruptibly();

        assertFalse(connection.isActive());
        assertFalse(connection.isClosed());

        ClientConnectionsEntry entry = new ClientConnectionsEntry(null, 0, 1, 0, 0,
                manager.proxy(), NodeType.MASTER);
        entry.releaseConnection(connection);

        RedisConnection reused = entry.pollConnection(null);
        assertSame(connection, reused);
        assertFalse(reused.isActive());
        assertFalse(reused.isClosed());
    }
}

TestConnectionManager is just a small test stub that provides getConnectionWatcher(), getConfig(), getGroup(), isShutdown(), and isShuttingDown().

Expected behavior

A connection whose channel is inactive should not be returned to or borrowed from the free connection pool.

Possible fixes:

1. ClientConnectionsEntry.releaseConnection() should check connection.isActive() before adding it back to freeConnections.
2. ClientConnectionsEntry.pollConnection() / ConnectionPool.connectTo() should discard inactive connections for master entries as well, not only slave entries.
3. The RedisExecutor branch that throws Command still hasn't been written into connection may need to close or force reconnect the connection before releasing it.

Actual behavior

An inactive master connection with RedisConnection.closed == false can be returned to the pool and borrowed again.