#147 parentCallStackSize not recorded safely
A user reported a problem with an assertion failure seen when using -stack:
Error stack trace:
0. /vesta/example.com/foo/build/generic_env/bar/bridge/checkout/1/13/build.ves: line 6, char 15
1. /vesta/vestasys.org/bridges/env_build/0/build.ves: line 448, char 9
vesta: /usr/include/Sequence.H:388: Elem Sequence<Elem, elem_ptrfree>::get(int) const [with Elem = ExprC*, bool elem_ptrfree = false]: Assertion `(0 <= i) && (i < sz)' failed.
Abort
At first I didn't have a core file which prompted me to do a quick code review, resulting in the small bug-fix in eval/110. That unfortunately didn't solve the problem, so I took a closer look and discovered that the problem had to do with the parentCallStackSize member variable of the ThreadData class.
When using -stack, each thread keeps its call stack in the callStack member variable of ThreadData. When using _par_map, the thread that executed the call to _par_map becomes the "parent" of the threads evaluating the different parallel calls. This association is recorded with the parent member variable of ThreadData.
When there are more parallel calls to process than there are threads to do the work, the parent thread that called _par_map will do some of that work. This will add more calls to the parent's ThreadData's callStack which should not be reported when showing an error call stack. So each child thread must know which portion of its parent thread's callStack should be printed. This is done using parentCallStackSize which should be the total number of entries in the parent's callStack when _par_map was called. Entries in callStack are added and removed at the beginning (using the "addlo" and "remlo" Sequence member functions), so the portion of the parents call stack which the child should include in an error stack is the parentCallStackSize elements at the end of the parent's callStack. The function PrintErrorStack in VASTi.C handles this across as many parent threads as it needs to traverse.
The problem is that parentCallStackSize is not always determined correctly. In some cases it is being set from the size of the parent thread's callStack after it may already be executing one of the parallel pieces of work.
The function that actually checks the size of the parent thread's callStack is StartWorker (Prim.C). From there it goes into the parentCallStackSize member variable in an instance of the EvalWorker class. When the thread actually starts this is propagated into the parentCallStackSize member variable of the ThreadData instance in the function DoWork (also in Prim.C).
StartWorker is called by the parent thread executing _par_map (in the function ParMap in Prim.C). However it is also called by a background thread which starts more threads to keep processing parallel work when the parent thread may be busy processing one of the units of work started by _par_map (in the function DoAvailWork in Prim.C). This can get the wrong value into parentCallStackSize. In the case I got a chance to look at closely, it was 7 but there were only 6 elements in the parent thread's callStack. In PrintErrorStack this had the variable "st" at -1, which is how the assertion was triggered.
The fix clearly involves at least:
- Passing the parent call stack size as an argument to StartWorker rather than having it determine it.
- Including the parent call stack size as a member variable in the ParWork class which is how information is sent to the DoAvailWork thread.
