From 263bb42091b7b62ebbc55dcdc96d5d19188a4d18 Mon Sep 17 00:00:00 2001 From: zy-kkk Date: Fri, 5 Dec 2025 14:02:30 +0800 Subject: [PATCH] [fix](jdbc) Fix JNI global reference leak in JdbcConnector close (#58574) ## Problem JNI Global References may not be deleted when `JdbcConnector::close()` fails midway due to early returns from `RETURN_IF_ERROR` macros. This prevents Java GC from collecting `BaseJdbcExecutor` and its associated `ResultSet` objects, causing old generation heap to grow to 99%+. ## Solution - C++: Always delete all three Global References regardless of Java close() failure - Java: Always clear member variable references (`resultSet`, `stmt`, `conn`) in finally block --- be/src/vec/exec/vjdbc_connector.cpp | 50 +++++++++++++++---- .../apache/doris/jdbc/BaseJdbcExecutor.java | 19 +++++-- 2 files changed, 54 insertions(+), 15 deletions(-) diff --git a/be/src/vec/exec/vjdbc_connector.cpp b/be/src/vec/exec/vjdbc_connector.cpp index d4cf31f4f78674..d55dd4b48e1055 100644 --- a/be/src/vec/exec/vjdbc_connector.cpp +++ b/be/src/vec/exec/vjdbc_connector.cpp @@ -68,23 +68,51 @@ JdbcConnector::~JdbcConnector() { Status JdbcConnector::close(Status /*unused*/) { SCOPED_RAW_TIMER(&_jdbc_statistic._connector_close_timer); - _closed = true; + if (_closed) { + return Status::OK(); + } if (!_is_open) { + _closed = true; return Status::OK(); } + + JNIEnv* env = nullptr; + Status status = JniUtil::GetJNIEnv(&env); + if (!status.ok() || env == nullptr) { + LOG(WARNING) << "Failed to get JNIEnv in close(): " << status.to_string(); + _closed = true; + return status; + } + + // Try to abort transaction and call Java close(), but don't block cleanup if (_is_in_transaction) { - RETURN_IF_ERROR(abort_trans()); + Status abort_status = abort_trans(); + if (!abort_status.ok()) { + LOG(WARNING) << "Failed to abort transaction: " << abort_status.to_string(); + } } - JNIEnv* env = nullptr; - RETURN_IF_ERROR(JniUtil::GetJNIEnv(&env)); + env->CallNonvirtualVoidMethod(_executor_obj, _executor_clazz, _executor_close_id); - RETURN_ERROR_IF_EXC(env); - env->DeleteGlobalRef(_executor_factory_clazz); - RETURN_ERROR_IF_EXC(env); - env->DeleteGlobalRef(_executor_clazz); - RETURN_IF_ERROR(JniUtil::GetJniExceptionMsg(env)); - env->DeleteGlobalRef(_executor_obj); - RETURN_ERROR_IF_EXC(env); + if (env->ExceptionCheck()) { + LOG(WARNING) << "Java close() failed: " << JniUtil::GetJniExceptionMsg(env).to_string(); + env->ExceptionClear(); + } + + // Always delete Global References to allow Java GC + if (_executor_factory_clazz != nullptr) { + env->DeleteGlobalRef(_executor_factory_clazz); + _executor_factory_clazz = nullptr; + } + if (_executor_clazz != nullptr) { + env->DeleteGlobalRef(_executor_clazz); + _executor_clazz = nullptr; + } + if (_executor_obj != nullptr) { + env->DeleteGlobalRef(_executor_obj); + _executor_obj = nullptr; + } + + _closed = true; return Status::OK(); } diff --git a/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java b/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java index 3e71cc5abddb0b..151e9cffe1af22 100644 --- a/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java +++ b/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java @@ -127,7 +127,11 @@ public BaseJdbcExecutor(byte[] thriftParams) throws Exception { public void close() throws Exception { if (outputTable != null) { - outputTable.close(); + try { + outputTable.close(); + } finally { + outputTable = null; + } } try { if (stmt != null && !stmt.isClosed()) { @@ -143,10 +147,17 @@ public void close() throws Exception { } } finally { closeResources(resultSet, stmt, conn); + // Always clear references to help GC, even if close() failed + resultSet = null; + stmt = null; + conn = null; if (config.getConnectionPoolMinSize() == 0 && hikariDataSource != null) { - hikariDataSource.close(); - JdbcDataSource.getDataSource().getSourcesMap().remove(config.createCacheKey()); - hikariDataSource = null; + try { + hikariDataSource.close(); + JdbcDataSource.getDataSource().getSourcesMap().remove(config.createCacheKey()); + } finally { + hikariDataSource = null; + } } } }