This is an automated email from the ASF dual-hosted git repository.
martinzink pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new 4ee6b4fe8 MINIFICPP-2887 Memory leak in
minifi_process_session_transfer/minifi_process_session_remove (#2249)
4ee6b4fe8 is described below
commit 4ee6b4fe8342d0058050c9be4ac78acf6e989990
Author: Martin Zink <[email protected]>
AuthorDate: Wed Aug 26 13:17:29 2026 +0200
MINIFICPP-2887 Memory leak in
minifi_process_session_transfer/minifi_process_session_remove (#2249)
---
.github/workflows/memcheck_ci.yml | 1 +
extensions/llamacpp/tests/CMakeLists.txt | 1 +
extensions/stable-api-testing/tests/CMakeLists.txt | 1 +
libminifi/src/minifi-api.cpp | 8 ++++--
libminifi/test/unit/ProcessSessionTests.cpp | 30 ++++++++++++++++++++++
5 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/.github/workflows/memcheck_ci.yml
b/.github/workflows/memcheck_ci.yml
index 3b8d0f71a..62192071f 100644
--- a/.github/workflows/memcheck_ci.yml
+++ b/.github/workflows/memcheck_ci.yml
@@ -9,6 +9,7 @@ env:
-DCI_BUILD=ON
-DDOCKER_BUILD_ONLY=OFF
-DENABLE_ALL=ON
+ -DENABLE_TEST_PROCESSORS=ON
-DMINIFI_FAIL_ON_WARNINGS=ON
-DPORTABLE=ON
-DUSE_SHARED_LIBS=ON
diff --git a/extensions/llamacpp/tests/CMakeLists.txt
b/extensions/llamacpp/tests/CMakeLists.txt
index d0f415a41..a9f267ab0 100644
--- a/extensions/llamacpp/tests/CMakeLists.txt
+++ b/extensions/llamacpp/tests/CMakeLists.txt
@@ -35,5 +35,6 @@ FOREACH(testfile ${LLAMACPP_TESTS})
MATH(EXPR EXTENSIONS_TEST_COUNT "${EXTENSIONS_TEST_COUNT}+1")
add_test(NAME ${testfilename} COMMAND ${testfilename} WORKING_DIRECTORY
${TEST_DIR})
+ set_tests_properties("${testfilename}" PROPERTIES LABELS
"llamacpp;memchecked")
ENDFOREACH()
message("-- Finished building ${EXTENSIONS_TEST_COUNT} llama.cpp related test
file(s)...")
diff --git a/extensions/stable-api-testing/tests/CMakeLists.txt
b/extensions/stable-api-testing/tests/CMakeLists.txt
index 55614119c..18e9d759e 100644
--- a/extensions/stable-api-testing/tests/CMakeLists.txt
+++ b/extensions/stable-api-testing/tests/CMakeLists.txt
@@ -32,4 +32,5 @@ FOREACH (testfile ${SOURCES})
MATH(EXPR EXTENSIONS_TEST_COUNT "${EXTENSIONS_TEST_COUNT}+1")
add_test(NAME ${testfilename} COMMAND ${testfilename} WORKING_DIRECTORY
${TEST_DIR})
+ set_tests_properties("${testfilename}" PROPERTIES LABELS "memchecked")
ENDFOREACH ()
diff --git a/libminifi/src/minifi-api.cpp b/libminifi/src/minifi-api.cpp
index d24cc96d8..0fae27f2b 100644
--- a/libminifi/src/minifi-api.cpp
+++ b/libminifi/src/minifi-api.cpp
@@ -456,8 +456,10 @@ minifi_status
minifi_process_session_penalize(minifi_process_session* session, m
minifi_status minifi_process_session_transfer(minifi_process_session* session,
MINIFI_OWNED minifi_flow_file* flowfile, minifi_string_view relationship_name) {
gsl_Assert(session);
gsl_Assert(flowfile);
+ // Adopt the MINIFI_OWNED handle: minifi takes ownership, so free the heap
std::shared_ptr<FlowFile> when this call returns.
+ const std::unique_ptr<std::shared_ptr<minifi::core::FlowFile>>
owned{toCpp(flowfile)};
try {
- toCpp(session)->transfer(*toCpp(flowfile),
minifi::core::Relationship{toString(relationship_name), ""});
+ toCpp(session)->transfer(*owned,
minifi::core::Relationship{toString(relationship_name), ""});
return MINIFI_STATUS_SUCCESS;
} catch (...) {
return MINIFI_STATUS_UNKNOWN_ERROR;
@@ -467,8 +469,10 @@ minifi_status
minifi_process_session_transfer(minifi_process_session* session, M
minifi_status minifi_process_session_remove(minifi_process_session* session,
MINIFI_OWNED minifi_flow_file* flowfile) {
gsl_Assert(session);
gsl_Assert(flowfile);
+ // Adopt the MINIFI_OWNED handle: minifi takes ownership, so free the heap
std::shared_ptr<FlowFile> when this call returns.
+ const std::unique_ptr<std::shared_ptr<minifi::core::FlowFile>>
owned{toCpp(flowfile)};
try {
- toCpp(session)->remove(*toCpp(flowfile));
+ toCpp(session)->remove(*owned);
return MINIFI_STATUS_SUCCESS;
} catch (...) {
return MINIFI_STATUS_UNKNOWN_ERROR;
diff --git a/libminifi/test/unit/ProcessSessionTests.cpp
b/libminifi/test/unit/ProcessSessionTests.cpp
index 5bd4d08ff..8c7e99bd6 100644
--- a/libminifi/test/unit/ProcessSessionTests.cpp
+++ b/libminifi/test/unit/ProcessSessionTests.cpp
@@ -26,6 +26,8 @@
#include "core/Processor.h"
#include "unit/TestUtils.h"
#include "core/repository/FileSystemRepository.h"
+#include "minifi-api.h"
+#include "utils/minifi-api-utils.h"
namespace {
@@ -136,3 +138,31 @@ TEST_CASE("Test ProcessSession::write's possible
outcomes") {
ContentRepositoryDependentTests::testErrWrite(std::make_shared<core::repository::FileSystemRepository>());
ContentRepositoryDependentTests::testCancelWrite(std::make_shared<core::repository::FileSystemRepository>());
}
+
+TEST_CASE("Stable C API does not leak the flow file handle",
"[minifi-api][flowfilehandle]") {
+ Fixture fixture;
+ auto& process_session = fixture.processSession();
+ auto* c_session = minifi::utils::toC(&process_session);
+
+ auto created = process_session.create();
+ process_session.transfer(created, Success);
+ process_session.commit();
+
+ // Track the underlying FlowFile object; drop every strong reference we hold
locally.
+ std::weak_ptr<minifi::core::FlowFile> weak_flow_file = created;
+ created.reset();
+
+ // Pull the flow file out through the stable C API. This does `new
std::shared_ptr<FlowFile>`
+ // behind the returned handle, so the handle owns one strong reference to
the FlowFile.
+ auto* handle = minifi_process_session_get(c_session);
+ REQUIRE(handle != nullptr);
+
+ // Hand the (MINIFI_OWNED) handle back to minifi and drop the flow file for
good.
+ REQUIRE(minifi_process_session_remove(c_session, handle) ==
MINIFI_STATUS_SUCCESS);
+ process_session.commit();
+
+ // After remove+commit no legitimate owner remains. If the C API failed to
free the heap
+ // std::shared_ptr<FlowFile> behind the handle, that leaked strong reference
keeps the
+ // FlowFile alive and this assertion fails.
+ REQUIRE(weak_flow_file.expired());
+}
