sahvx655-wq commented on code in PR #707:
URL: https://github.com/apache/logging-log4cxx/pull/707#discussion_r3367529584
##########
src/main/cpp/threadutility.cpp:
##########
@@ -369,39 +369,86 @@ void ThreadUtility::priv_data::doPeriodicTasks()
{
auto currentTime = std::chrono::system_clock::now();
TimePoint nextOperationTime = currentTime + this->maxDelay;
+
+ while (true)
{
- std::lock_guard<std::recursive_mutex>
lock(this->job_mutex);
- for (auto& item : this->jobs)
+ std::function<void()> taskCallback;
+ LogString taskName;
+ Period taskDelay;
+ bool foundTask = false;
+
{
- if (this->terminated.load())
- return;
- if (item.removed)
- ;
- else if (item.nextRun <= currentTime)
+ std::lock_guard<std::recursive_mutex>
lock(this->job_mutex);
+ for (auto& item : this->jobs)
{
- try
+ if (this->terminated.load())
+ return;
+
+ if (!item.removed && item.nextRun <=
currentTime)
{
- item.f();
- item.nextRun =
std::chrono::system_clock::now() + item.delay;
- if (item.nextRun <
nextOperationTime)
- nextOperationTime =
item.nextRun;
- item.errorCount = 0;
+ taskCallback = item.f;
+ taskName = item.name;
+ taskDelay = item.delay;
+ foundTask = true;
+ break;
}
- catch (std::exception& ex)
+ }
+ }
+
+ if (!foundTask)
+ {
+ break;
+ }
+
+ bool success = false;
+ try
+ {
+ taskCallback();
+ success = true;
+ }
+ catch (std::exception& ex)
+ {
+ LogLog::warn(taskName, ex);
+ }
+ catch (...)
+ {
+ LogLog::warn(taskName + LOG4CXX_STR(" threw an
exception"));
+ }
+
+ {
+ std::lock_guard<std::recursive_mutex>
lock(this->job_mutex);
+ auto pItem = std::find_if(this->jobs.begin(),
this->jobs.end()
+ , [&taskName](const
NamedPeriodicFunction& item)
+ { return !item.removed && taskName ==
item.name; }
+ );
+
+ if (pItem != this->jobs.end())
+ {
+ if (success)
{
- LogLog::warn(item.name, ex);
- ++item.errorCount;
+ pItem->nextRun =
std::chrono::system_clock::now() + taskDelay;
+ pItem->errorCount = 0;
Review Comment:
Done. nextRun now gets pushed out on both paths, so a task that keeps
failing (a reconnect that can't reach the server) waits taskDelay before the
next attempt instead of spinning. errorCount still only resets on success.
##########
src/main/cpp/threadutility.cpp:
##########
@@ -369,39 +369,86 @@ void ThreadUtility::priv_data::doPeriodicTasks()
{
auto currentTime = std::chrono::system_clock::now();
TimePoint nextOperationTime = currentTime + this->maxDelay;
+
+ while (true)
{
- std::lock_guard<std::recursive_mutex>
lock(this->job_mutex);
- for (auto& item : this->jobs)
+ std::function<void()> taskCallback;
+ LogString taskName;
+ Period taskDelay;
+ bool foundTask = false;
+
{
- if (this->terminated.load())
- return;
- if (item.removed)
- ;
- else if (item.nextRun <= currentTime)
+ std::lock_guard<std::recursive_mutex>
lock(this->job_mutex);
+ for (auto& item : this->jobs)
Review Comment:
Switched the selection loop over to std::find_if as well, so it matches the
reschedule and cleanup blocks. The terminated check now sits just before we
take the lock.
--
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]
