Skip to content

ConnectionDecorator shows use the target Connection for equality checks  #77

@igorlovich

Description

@igorlovich

For reference Spring's JdbcTemplate.queryForStream method

	@Override
	public <T> Stream<T> queryForStream(String sql, RowMapper<T> rowMapper) throws DataAccessException {
		class StreamStatementCallback implements StatementCallback<Stream<T>>, SqlProvider {
			@Override
			public Stream<T> doInStatement(Statement stmt) throws SQLException {
				ResultSet rs = stmt.executeQuery(sql);
				Connection con = stmt.getConnection();
				return new ResultSetSpliterator<>(rs, rowMapper).stream().onClose(() -> {
					JdbcUtils.closeResultSet(rs);
					JdbcUtils.closeStatement(stmt);
					DataSourceUtils.releaseConnection(con, getDataSource());
				});
			}
			@Override
			public String getSql() {
				return sql;
			}
		}

		return result(execute(new StreamStatementCallback(), false));
	}

This method obtains a connection from a Statement. Since FlexyPool does not proxy Statements (it returns the actual Statement from the delegate), stmnt.getConnection() returns the actual datasource connection (HikariConnection in my case) and not a ConnectionDecorator.

In the onClose the DataSourceUtils#doReleaseConnection

	public static void doReleaseConnection(@Nullable Connection con, @Nullable DataSource dataSource) throws SQLException {
		if (con == null) {
			return;
		}
		if (dataSource != null) {
			ConnectionHolder conHolder = (ConnectionHolder) TransactionSynchronizationManager.getResource(dataSource);
			if (conHolder != null && connectionEquals(conHolder, con)) {
				// It's the transactional Connection: Don't close it.
				conHolder.released();
				return;
			}
		}
		doCloseConnection(con, dataSource);
	}

calls DataSourceUtils#connectionEquals that performs and equality check on the passed in connection and the connection being held by the transaction.

	private static boolean connectionEquals(ConnectionHolder conHolder, Connection passedInCon) {
		if (!conHolder.hasConnection()) {
			return false;
		}
		Connection heldCon = conHolder.getConnection();
		// Explicitly check for identity too: for Connection handles that do not implement
		// "equals" properly, such as the ones Commons DBCP exposes).
		return (heldCon == passedInCon || heldCon.equals(passedInCon) ||
				getTargetConnection(heldCon).equals(passedInCon));
	}

The connection held by the transaction is a ConnectionDecorator and the equality check fails. The result is the instead of decrementing the reference count on the transaction resource, the connection is marked as closed and when the transaction actually finishes and tries to commit it throws and exception as the connection is "closed"

Would it make sense to override the equals / hashCode method in ConnectionDecorator with something like this

    @Override
    public boolean equals(Object obj) {
        if (this == obj) {
            return true;
        }
        if(obj instanceof ConnectionDecorator decorator) {
            return getTarget().equals(decorator.getTarget());
        }
        return getTarget().equals(obj);
    }

    @Override
    public int hashCode() {
        return getTarget().hashCode();
    }

I can open a PR if this seems reasonable

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions