Does moving leave the object in a usable state? Does moving leave the object in a usable state?
According to this your m_Queue object is valid, yet in an unspecified state.
The easiest way to make the state specified again might be calling m_Queue.clear() directly after local_queue = std::move(m_Queue); or even reinitializing m_Queue with new object.
I tried to find out whether just clearing m_Queue is sufficient, yet I couldn't find anything definate apart from comments. I think it is at least "more safe" to use than you current approach.
For readability reasons I would have also changed the structure of your private_start to something like this
void private_start()
{
m_Thread = std::thread([this]
{
for (;;)
{
decltype(m_Queue) local_queue;
{
std::unique_lock<std::mutex> lk(m_Mutex);
m_Condition.wait(lk, [&] { return !m_Queue.empty() + !m_Running; });
if (!m_Running)
{
for (auto& func : m_Queue)
func();
m_Queue.clear();
return;
}
//Move to local
local_queue = std::move(m_Queue);
m_Queue.clear();
}
//Process local
for (auto& func : local_queue)
{
func();
}
}
});
}
But that's more of a personnal preference.
As a last comment I would recommend updating your example to call stop on all workers so your application doesn't end prematurely.
Does moving leave the object in a usable state?
According to this your m_Queue object is valid, yet in an unspecified state.
The easiest way to make the state specified again might be calling m_Queue.clear() directly after local_queue = std::move(m_Queue); or even reinitializing m_Queue with new object.
I tried to find out whether just clearing m_Queue is sufficient, yet I couldn't find anything definate apart from comments. I think it is at least "more safe" to use than you current approach.
For readability reasons I would have also changed the structure of your private_start to something like this
void private_start()
{
m_Thread = std::thread([this]
{
for (;;)
{
decltype(m_Queue) local_queue;
{
std::unique_lock<std::mutex> lk(m_Mutex);
m_Condition.wait(lk, [&] { return !m_Queue.empty() + !m_Running; });
if (!m_Running)
{
for (auto& func : m_Queue)
func();
m_Queue.clear();
return;
}
//Move to local
local_queue = std::move(m_Queue);
m_Queue.clear();
}
//Process local
for (auto& func : local_queue)
{
func();
}
}
});
}
But that's more of a personnal preference.
As a last comment I would recommend updating your example to call stop on all workers so your application doesn't end prematurely.
Does moving leave the object in a usable state?
According to this your m_Queue object is valid, yet in an unspecified state.
The easiest way to make the state specified again might be calling m_Queue.clear() directly after local_queue = std::move(m_Queue); or even reinitializing m_Queue with new object.
I tried to find out whether just clearing m_Queue is sufficient, yet I couldn't find anything definate apart from comments. I think it is at least "more safe" to use than you current approach.
For readability reasons I would have also changed the structure of your private_start to something like this
void private_start()
{
m_Thread = std::thread([this]
{
for (;;)
{
decltype(m_Queue) local_queue;
{
std::unique_lock<std::mutex> lk(m_Mutex);
m_Condition.wait(lk, [&] { return !m_Queue.empty() + !m_Running; });
if (!m_Running)
{
for (auto& func : m_Queue)
func();
m_Queue.clear();
return;
}
//Move to local
local_queue = std::move(m_Queue);
m_Queue.clear();
}
//Process local
for (auto& func : local_queue)
{
func();
}
}
});
}
But that's more of a personnal preference.
As a last comment I would recommend updating your example to call stop on all workers so your application doesn't end prematurely.
Does moving leave the object in a usable state?
According to this your m_Queue object is valid, yet in an unspecified state.
The easiest way to make the state specified again might be calling m_Queue.clear() directly after local_queue = std::move(m_Queue); or even reinitializing m_Queue with new object.
I tried to find out whether just clearing m_Queue is sufficient, yet I couldn't find anything definate apart from comments. I think it is at least "more safe" to use than you current approach.
For readability reasons I would have also changed the structure of your private_start to something like this
void private_start()
{
m_Thread = std::thread([this]
{
for (;;)
{
decltype(m_Queue) local_queue;
{
std::unique_lock<std::mutex> lk(m_Mutex);
m_Condition.wait(lk, [&] { return !m_Queue.empty() + !m_Running; });
if (!m_Running)
{
for (auto& func : m_Queue)
func();
m_Queue.clear();
return;
}
//Move to local
local_queue = std::move(m_Queue);
m_Queue.clear();
}
//Process local
for (auto& func : local_queue)
{
func();
}
}
});
}
But that's more of a personnal preference.
As a last comment I would recommend updating your example to call stop on all workers so your application doesn't end prematurely.