HHH-15403 Likely Statement leak on invoking a stored procedure

This commit is contained in:
Andrea Boriero 2022-07-20 10:32:45 +02:00 committed by Sanne Grinovero
parent a4e52f91f8
commit 1f31284f33
6 changed files with 113 additions and 86 deletions

View File

@ -613,81 +613,78 @@ public class ProcedureCallImpl<R>
.getJdbcCoordinator() .getJdbcCoordinator()
.getStatementPreparer() .getStatementPreparer()
.prepareStatement( call.getSql(), true ); .prepareStatement( call.getSql(), true );
// Register the parameter mode and type
callableStatementSupport.registerParameters(
procedureName,
call,
statement,
parameterMetadata,
getSession()
);
// Apply the parameter bindings
final JdbcParameterBindings jdbcParameterBindings = new JdbcParameterBindingsImpl( parameterRegistrations.size() );
for ( Map.Entry<ProcedureParameter<?>, JdbcCallParameterRegistration> entry : parameterRegistrations.entrySet() ) {
final JdbcCallParameterRegistration registration = entry.getValue();
if ( registration.getParameterBinder() != null ) {
final ProcedureParameter<?> parameter = entry.getKey();
final QueryParameterBinding<?> binding = getParameterBindings().getBinding( parameter );
if ( !binding.isBound() ) {
if ( parameter.getPosition() == null ) {
throw new IllegalArgumentException( "The parameter named [" + parameter + "] was not set! You need to call the setParameter method." );
}
else {
throw new IllegalArgumentException( "The parameter at position [" + parameter + "] was not set! You need to call the setParameter method." );
}
}
jdbcParameterBindings.addBinding(
(JdbcParameter) registration.getParameterBinder(),
new JdbcParameterBindingImpl(
(JdbcMapping) registration.getParameterType(),
binding.getBindValue()
)
);
}
}
final JdbcCallRefCursorExtractor[] extractors = refCursorExtractors.toArray( new JdbcCallRefCursorExtractor[0] );
final ExecutionContext executionContext = new ExecutionContext() {
private final Callback callback = new CallbackImpl();
@Override
public SharedSessionContractImplementor getSession() {
return ProcedureCallImpl.this.getSession();
}
@Override
public QueryOptions getQueryOptions() {
return new QueryOptionsAdapter() {
@Override
public Boolean isReadOnly() {
return false;
}
};
}
@Override
public String getQueryIdentifier(String sql) {
return sql;
}
@Override
public QueryParameterBindings getQueryParameterBindings() {
return QueryParameterBindings.NO_PARAM_BINDINGS;
}
@Override
public Callback getCallback() {
return callback;
}
};
// Note that this should actually happen in an executor
try { try {
// Register the parameter mode and type
callableStatementSupport.registerParameters(
procedureName,
call,
statement,
parameterMetadata,
getSession()
);
// Apply the parameter bindings
final JdbcParameterBindings jdbcParameterBindings = new JdbcParameterBindingsImpl( parameterRegistrations.size() );
for ( Map.Entry<ProcedureParameter<?>, JdbcCallParameterRegistration> entry : parameterRegistrations.entrySet() ) {
final JdbcCallParameterRegistration registration = entry.getValue();
if ( registration.getParameterBinder() != null ) {
final ProcedureParameter<?> parameter = entry.getKey();
final QueryParameterBinding<?> binding = getParameterBindings().getBinding( parameter );
if ( !binding.isBound() ) {
if ( parameter.getPosition() == null ) {
throw new IllegalArgumentException( "The parameter named [" + parameter + "] was not set! You need to call the setParameter method." );
}
else {
throw new IllegalArgumentException( "The parameter at position [" + parameter + "] was not set! You need to call the setParameter method." );
}
}
jdbcParameterBindings.addBinding(
(JdbcParameter) registration.getParameterBinder(),
new JdbcParameterBindingImpl(
(JdbcMapping) registration.getParameterType(),
binding.getBindValue()
)
);
}
}
final ExecutionContext executionContext = new ExecutionContext() {
private final Callback callback = new CallbackImpl();
@Override
public SharedSessionContractImplementor getSession() {
return ProcedureCallImpl.this.getSession();
}
@Override
public QueryOptions getQueryOptions() {
return new QueryOptionsAdapter() {
@Override
public Boolean isReadOnly() {
return false;
}
};
}
@Override
public String getQueryIdentifier(String sql) {
return sql;
}
@Override
public QueryParameterBindings getQueryParameterBindings() {
return QueryParameterBindings.NO_PARAM_BINDINGS;
}
@Override
public Callback getCallback() {
return callback;
}
};
// Note that this should actually happen in an executor
int paramBindingPosition = call.getFunctionReturn() == null ? 1 : 2; int paramBindingPosition = call.getFunctionReturn() == null ? 1 : 2;
for ( JdbcParameterBinder parameterBinder : call.getParameterBinders() ) { for ( JdbcParameterBinder parameterBinder : call.getParameterBinders() ) {
parameterBinder.bindParameterValue( parameterBinder.bindParameterValue(
@ -700,13 +697,21 @@ public class ProcedureCallImpl<R>
} }
} }
catch (SQLException e) { catch (SQLException e) {
getSession().getJdbcCoordinator().getLogicalConnection().getResourceRegistry().release( statement );
throw getSession().getJdbcServices().getSqlExceptionHelper().convert( throw getSession().getJdbcServices().getSqlExceptionHelper().convert(
e, e,
"Error registering CallableStatement parameters", "Error registering CallableStatement parameters",
procedureName procedureName
); );
} }
return new ProcedureOutputsImpl( this, parameterRegistrations, extractors, statement );
return new ProcedureOutputsImpl(
this,
parameterRegistrations,
refCursorExtractors.toArray( new JdbcCallRefCursorExtractor[0] ),
statement
);
} }
@Override @Override

View File

@ -148,4 +148,9 @@ public class ProcedureOutputsImpl extends OutputsImpl implements ProcedureOutput
} }
} }
@Override
public void release() {
super.release();
getResultContext().getSession().getJdbcCoordinator().getLogicalConnection().getResourceRegistry().release( callableStatement );
}
} }

View File

@ -133,7 +133,14 @@ public final class ResourceRegistryStandardImpl implements ResourceRegistry {
else { else {
resultSets.remove( resultSet ); resultSets.remove( resultSet );
if ( resultSets.isEmpty() ) { if ( resultSets.isEmpty() ) {
xref.remove( statement ); try {
if ( statement.isClosed() ) {
xref.remove( statement );
}
}
catch (SQLException e) {
log.debugf( "Unable to release JDBC statement [%s]", e.getMessage() );
}
} }
} }
} }

View File

@ -67,6 +67,10 @@ public class OutputsImpl implements Outputs {
} }
protected ResultContext getResultContext(){
return context;
}
protected void executeStatement() { protected void executeStatement() {
try { try {
final boolean isResultSet = jdbcStatement.execute(); final boolean isResultSet = jdbcStatement.execute();
@ -134,12 +138,7 @@ public class OutputsImpl implements Outputs {
@Override @Override
public void release() { public void release() {
try { context.getSession().getJdbcCoordinator().getLogicalConnection().getResourceRegistry().release( jdbcStatement );
jdbcStatement.close();
}
catch (SQLException e) {
log.debug( "Unable to close PreparedStatement", e );
}
} }
private List extractCurrentResults() { private List extractCurrentResults() {
@ -281,9 +280,6 @@ public class OutputsImpl implements Outputs {
} }
return results; return results;
} }
// catch (SQLException e) {
// throw context.getSession().getExceptionConverter().convert( e, "Error processing return rows" );
// }
finally { finally {
rowReader.finishUp( jdbcValuesSourceProcessingState ); rowReader.finishUp( jdbcValuesSourceProcessingState );
jdbcValuesSourceProcessingState.finishUp(); jdbcValuesSourceProcessingState.finishUp();

View File

@ -10,6 +10,7 @@ import java.sql.Connection;
import java.sql.PreparedStatement; import java.sql.PreparedStatement;
import java.sql.SQLException; import java.sql.SQLException;
import java.util.Arrays; import java.util.Arrays;
import java.util.List;
import org.hibernate.Session; import org.hibernate.Session;
import org.hibernate.dialect.OracleDialect; import org.hibernate.dialect.OracleDialect;
@ -18,6 +19,7 @@ import org.hibernate.engine.jdbc.spi.ResultSetReturn;
import org.hibernate.engine.jdbc.spi.StatementPreparer; import org.hibernate.engine.jdbc.spi.StatementPreparer;
import org.hibernate.engine.spi.SessionImplementor; import org.hibernate.engine.spi.SessionImplementor;
import org.hibernate.jdbc.Work; import org.hibernate.jdbc.Work;
import org.hibernate.procedure.ProcedureCall;
import org.hibernate.testing.RequiresDialect; import org.hibernate.testing.RequiresDialect;
import org.hibernate.testing.TestForIssue; import org.hibernate.testing.TestForIssue;
@ -84,15 +86,19 @@ public class CursorFromCallableTest extends BaseCoreFunctionalTestCase {
// Verify statement closing with JdbcCoordinator#hasRegisteredResources() instead. // Verify statement closing with JdbcCoordinator#hasRegisteredResources() instead.
// BigDecimal maxCursors = (BigDecimal) session.createSQLQuery( "SELECT value FROM v$parameter WHERE name = 'open_cursors'" ).uniqueResult(); // BigDecimal maxCursors = (BigDecimal) session.createSQLQuery( "SELECT value FROM v$parameter WHERE name = 'open_cursors'" ).uniqueResult();
// for ( int i = 0; i < maxCursors + 10; ++i ) { named_query_execution } // for ( int i = 0; i < maxCursors + 10; ++i ) { named_query_execution }
ProcedureCall namedStoredProcedureQuery = session.createNamedStoredProcedureQuery( "NumValue.getSomeValues" );
List resultList = namedStoredProcedureQuery.getResultList();
Assert.assertEquals( Assert.assertEquals(
Arrays.asList( new NumValue( 1, "Line 1" ), new NumValue( 2, "Line 2" ) ), Arrays.asList( new NumValue( 1, "Line 1" ), new NumValue( 2, "Line 2" ) ),
session.createNamedStoredProcedureQuery( "NumValue.getSomeValues" ).getResultList() resultList
); );
namedStoredProcedureQuery.close();
JdbcCoordinator jdbcCoordinator = ( (SessionImplementor) session ).getJdbcCoordinator(); JdbcCoordinator jdbcCoordinator = ( (SessionImplementor) session ).getJdbcCoordinator();
Assert.assertFalse( Assert.assertFalse(
"Prepared statement and result set should be released after query execution.", "Prepared statement and result set should be released after query execution.",
jdbcCoordinator.getLogicalConnection().getResourceRegistry().hasRegisteredResources() jdbcCoordinator.getLogicalConnection().getResourceRegistry().hasRegisteredResources()
); );
session.getTransaction().commit(); session.getTransaction().commit();
session.close(); session.close();
} }

View File

@ -119,6 +119,14 @@ public class PreparedStatementSpyConnectionProvider extends ConnectionProviderDe
return statementSpy; return statementSpy;
} ).when( connectionSpy ).prepareStatement( ArgumentMatchers.anyString() ); } ).when( connectionSpy ).prepareStatement( ArgumentMatchers.anyString() );
Mockito.doAnswer( invocation -> {
PreparedStatement statement = (PreparedStatement) invocation.callRealMethod();
PreparedStatement statementSpy = spy( statement, settingsForStatements );
String sql = (String) invocation.getArguments()[0];
preparedStatementMap.put( statementSpy, sql );
return statementSpy;
} ).when( connectionSpy ).prepareCall( ArgumentMatchers.anyString() );
Mockito.doAnswer( invocation -> { Mockito.doAnswer( invocation -> {
Statement statement = (Statement) invocation.callRealMethod(); Statement statement = (Statement) invocation.callRealMethod();
Statement statementSpy = spy( statement, settingsForStatements ); Statement statementSpy = spy( statement, settingsForStatements );