I'm trying to isolate a multithreading issue in my C++ application.
The Handler below gets created and "processed" within an auxiliary thread.
struct Handler
{
void process(const std::vector<size_t> ops)
{
std::vector<size_t>::const_iterator iter = ops.cbegin();
while( iter != ops.cend() ) {
processOperation( op );
}
}
void processOperation(std::vector<size_t>::const_iterator& iter)
{
size_t op = *iter;
iter++;
switch( op ) {
case frame_push : {
processOperation( iter );
break;
}
case frame_pop : {
return;
}
default : { break; }
}
}
};
As you can see in the above, processOperation() calls itself recursively if the current "op" equals frame_push.
I'm aware that sufficient recursive operations here would cause the call stack to overflow. However, when I run this Handler on an auxiliary thread instead of the main thread, that seems to happen much more quickly.
It seems like the possibility of a call stack overflow is not my only issue here. Might there be a reentrancy problem as well?
I would really appreciate any insights. Thanks!
First thing you've successfully eliminated is ownership of "ops" since you pass it by value, so we are dealing with a thread-local copy:
void process(const std::vector<size_t> ops)
Then you create an iterator
std::vector<size_t>::const_iterator iter = ops.cbegin();
Which you could probably write as:
auto iter = ops.begin();
but ok. Now we set our loop constraint:
while( iter != ops.cend() ) {
This however, it outside of the recursive call. So it will only be checked when we leave the recursive call.
void processOperation(std::vector<size_t>::const_iterator& iter)
{
size_t op = *iter;
iter++;
switch( op ) {
case frame_push : {
processOperation( iter );
break;
}
case frame_pop : {
return;
}
default : { break; }
}
}
There is no code in this call for the end of the container. I'm guessing that frame_push and frame_pop is to allow you to create scopes for whatever your default case would be replaced with, hence your need for recursion.
If that's the intent, it might be better to implement your own frame stack.
#include <vector>
#include <iostream>
#include <thread>
enum Operands { frame_push, frame_pop, other };
struct Frame {};
struct Handler
{
size_t maxDepth, pushCount, popCount;
Handler() : maxDepth(0), pushCount(0), popCount(0) {}
void handle(const std::vector<Operands> ops_)
{
std::vector<Frame> stack;
stack.reserve(256);
stack.emplace_back(); // Create a default Frame
for (auto iter = ops_.cbegin(); iter != ops_.cend(); ++iter) {
switch (*iter) {
case frame_push:
// make a copy of the current frame,
// remove the 'stack.back()' if new frames should
// start empty
++pushCount;
stack.emplace_back(stack.back());
maxDepth = std::max(maxDepth, stack.size());
break;
case frame_pop:
stack.pop_back();
++popCount;
break;
default: {
Frame& frame = stack.back();
(void)frame;
}
break;
}
}
std::this_thread::yield();
std::cout << "ops len " << ops_.size()
<< ", pushCount " << pushCount
<< ", popCount " << popCount
<< ", maxDepth " << maxDepth
<< std::endl;
std::this_thread::yield();
}
};
int main()
{
std::vector<Operands> ops = {
frame_push, other, other,
frame_push, other, frame_pop,
other,
frame_push, frame_pop,
other,
frame_push,
frame_push, other, other, frame_pop,
frame_pop,
frame_pop
};
Handler h;
std::thread t1(&Handler::handle, &h, ops);
t1.join();
}