面试题：连接池实现优化——重写close方法回收连接

Great question! Your core approach here is spot-on—using a decorator pattern to wrap the third-party Connection and override its close() method to return the connection to the pool instead of closing it. That's the standard industry practice for connection pooling, so you're on the right track. Let's break down the correctness of your implementation first, then look at improvements.

Is Your Implementation Correct? #

Your code has the right idea, but there are critical bugs and potential issues that need fixing:

• Fatal NPE Bug: Your MyConnection constructor doesn't assign the connectionPool parameter to the instance field. This means when close() is called, it'll throw a NullPointerException immediately. This is an easy fix but essential.
• Thread Safety Risks: You're using LinkedList as your connection queue, which is not thread-safe. In a multi-threaded environment (which is almost always the case for connection pools), concurrent calls to getConnection() and closeConnection() could cause race conditions, lost connections, or ConcurrentModificationExceptions.
• Visibility of isClosed: The isClosed flag isn't marked with volatile, so changes to its value might not be visible across threads. This could lead to duplicate returns to the pool or attempts to use already-closed connections.
• Empty Pool Handling: If the pool runs out of connections, connections.poll() returns null, and you create a MyConnection with a null underlying connection. This will cause an NPE when the client calls execute().
• Missing Cleanup Logic: There's no way to properly destroy real connections (e.g., when the pool shuts down, or a connection becomes invalid). Your current code only returns wrapped connections to the queue, but you never call the third-party Connection's actual close() method when needed.

Optimized Implementation #

Here's a refined version of your code that fixes these issues and adds best practices for connection pooling:

import java.util.List;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.NoSuchElementException;

interface Connection {
    void execute();
    void close();
}

interface ConnectionPool {
    Connection getConnection();
    // Add method to shut down the pool and clean up all real connections
    void shutdown();
}

class Client {
    ConnectionPool connectionPool = new MyConnectionPool(List.of(new RealConnection()));
    Connection connection = connectionPool.getConnection();

    public void execute() {
        try {
            connection.execute();
        } finally {
            connection.close();
        }
    }
}

class MyConnectionPool implements ConnectionPool {
    // Separate queues for real (unused) connections and wrapped (pooled) connections
    private final Queue<Connection> realConnections;
    private final Queue<MyConnection> pooledConnections;

    MyConnectionPool(List<Connection> connections) {
        this.realConnections = new ConcurrentLinkedQueue<>(connections);
        this.pooledConnections = new ConcurrentLinkedQueue<>();
    }

    @Override
    public Connection getConnection() {
        // First try to reuse an existing wrapped connection
        MyConnection reusedConn = pooledConnections.poll();
        if (reusedConn != null) {
            reusedConn.reset();
            return reusedConn;
        }

        // If no reused connections, take a new real connection
        Connection realConn = realConnections.poll();
        if (realConn == null) {
            throw new NoSuchElementException("No available connections in the pool");
        }
        return new MyConnection(realConn, this);
    }

    void returnConnection(MyConnection connection) {
        pooledConnections.add(connection);
    }

    @Override
    public void shutdown() {
        // Clean up all real connections to prevent resource leaks
        realConnections.forEach(Connection::close);
        pooledConnections.stream()
                .map(MyConnection::getRealConnection)
                .forEach(Connection::close);
        
        realConnections.clear();
        pooledConnections.clear();
    }

    // Private inner class to enforce encapsulation—only the pool can create these
    private static class MyConnection implements Connection {
        private final Connection realConnection;
        private final MyConnectionPool pool;
        private volatile boolean isClosed = true; // Start in closed state

        MyConnection(Connection realConnection, MyConnectionPool pool) {
            this.realConnection = realConnection;
            this.pool = pool; // Fixed the NPE bug here!
        }

        // Reset state when reusing the connection
        void reset() {
            isClosed = false;
        }

        // Expose real connection for shutdown cleanup
        Connection getRealConnection() {
            return realConnection;
        }

        @Override
        public void execute() {
            if (isClosed) {
                throw new IllegalStateException("Cannot execute on a closed connection");
            }
            realConnection.execute();
        }

        @Override
        public void close() {
            if (!isClosed) {
                isClosed = true;
                pool.returnConnection(this);
            }
        }
    }
}

// Example third-party Connection implementation
class RealConnection implements Connection {
    @Override
    public void execute() {
        System.out.println("Executing real connection logic");
    }

    @Override
    public void close() {
        System.out.println("Closing real connection (cleanup)");
    }
}

Key Improvements: #

1. Fixed NPE Bug: Properly assigns the pool reference in MyConnection's constructor.
2. Thread Safety: Uses ConcurrentLinkedQueue for all queue operations, ensuring safe concurrent access without explicit locks.
3. Volatile Flag: isClosed is marked volatile to guarantee visibility across threads.
4. Better Pool Management: Separates real connections from wrapped pooled connections, making shutdown and cleanup easier.
5. Error Handling: Throws a clear exception when the pool is empty instead of returning a broken connection.
6. Encapsulation: MyConnection is a private static inner class, so it can't be instantiated outside the pool, enforcing correct usage.
7. Shutdown Logic: Adds a shutdown() method to properly close all real connections when the pool is no longer needed, preventing resource leaks.

Optional Extra Optimizations #

For production-grade pools, you could add:

• Bounded Pool: Use LinkedBlockingQueue with a fixed capacity to limit the number of concurrent connections.
• Connection Validation: Check if a connection is still valid (e.g., ping a database) before returning it to a client.
• Timeout Handling: Automatically destroy connections that have been idle for too long, or let clients wait for a connection with a timeout.

内容的提问来源于stack exchange，提问作者robin