Copilot commented on code in PR #2009:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2009#discussion_r2276056263
##########
extensions/expression-language/tests/ProcessContextExprTests.cpp:
##########
@@ -129,3 +129,28 @@ TEST_CASE("ProcessContextExpr can use expression language
in dynamic properties"
}
}
}
+
+TEST_CASE("ProcessContextExpr is mutex guarded properly") {
+ TestController test_controller;
+ const std::shared_ptr<TestPlan> test_plan = test_controller.createPlan();
+ std::ignore = test_plan->addProcessor("DummyProcessor", "dummy_processor");
+ test_plan->runNextProcessor();
+ const auto context = test_plan->getCurrentContext();
+ REQUIRE(dynamic_pointer_cast<core::ProcessContextExpr>(context) != nullptr);
+
+ auto play_with_context = [=]() {
+ for (auto i = 0; i < 100; ++i) {
+ context->setDynamicProperty("foo", fmt::format("${{literal('{}')}}",
std::this_thread::get_id()));
+ const auto dynamic_properties = context->getDynamicProperties();
+ std::this_thread::sleep_for(std::chrono::milliseconds(10));
+ }
+ };
+
+ std::thread thread_one{[&] { play_with_context(); }};
+ std::thread thread_two{[&] { play_with_context(); }};
+ std::thread thread_three{[&] { play_with_context(); }};
Review Comment:
[nitpick] The lambda captures by reference but calls 'play_with_context()'
which was already defined with its own capture. This creates an unnecessary
lambda wrapper. Consider using 'std::thread thread_one{play_with_context};'
directly.
```suggestion
std::thread thread_one{play_with_context};
std::thread thread_two{play_with_context};
std::thread thread_three{play_with_context};
```
##########
extensions/expression-language/tests/ProcessContextExprTests.cpp:
##########
@@ -129,3 +129,28 @@ TEST_CASE("ProcessContextExpr can use expression language
in dynamic properties"
}
}
}
+
+TEST_CASE("ProcessContextExpr is mutex guarded properly") {
+ TestController test_controller;
+ const std::shared_ptr<TestPlan> test_plan = test_controller.createPlan();
+ std::ignore = test_plan->addProcessor("DummyProcessor", "dummy_processor");
+ test_plan->runNextProcessor();
+ const auto context = test_plan->getCurrentContext();
+ REQUIRE(dynamic_pointer_cast<core::ProcessContextExpr>(context) != nullptr);
+
+ auto play_with_context = [=]() {
Review Comment:
The lambda captures all variables by value '[=]', which creates unnecessary
copies including the shared_ptr context. Consider capturing by reference '[&]'
or explicitly capture only what's needed '[context]' to avoid unnecessary
copying.
```suggestion
auto play_with_context = [&context]() {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
