Open wwmayer opened 4 years ago
Thanks for the thorough issue report, it is highly appreciated. I was able to reproduce the faulty behavior, but need some time to come up with a proper fix.
I think I spotted the problem :
So using some stacks for those two variables seems to do the trick :
GLState * postcachestate;
GLState * precachestate;
SbPList postcachestateList;
SbPList precachestateList;
GLState * postcachestate() const {
int len = postcachestateList.getLength();
if (len == 0) {
return nullptr;
}
return (GLState*)postcachestateList.get(len - 1);
}
GLState * precachestate() const {
int len = precachestateList.getLength();
if (len == 0) {
return nullptr;
}
return (GLState*)precachestateList.get(len - 1);
}
void
SoGLLazyElement::push(SoState * stateptr)
{
inherited::push(stateptr);
SoGLLazyElement * prev = (SoGLLazyElement*) this->getNextInStack();
this->state = stateptr; // needed to send GL texture
this->glstate = prev->glstate;
this->colorindex = prev->colorindex;
this->transpmask = prev->transpmask;
this->colorpacker = prev->colorpacker;
this->precachestateList = prev->precachestateList;
this->postcachestateList = prev->postcachestateList;
this->didsetbitmask = prev->didsetbitmask;
this->didntsetbitmask = prev->didntsetbitmask;
this->cachebitmask = prev->cachebitmask;
this->opencacheflags = prev->opencacheflags;
}
void
SoGLLazyElement::beginCaching(SoState * state, GLState * prestate,
GLState * poststate)
{
SoGLLazyElement * elem = getInstance(state);
elem->send(state, ALL_MASK); // send lazy state before starting to build cache
*prestate = elem->glstate; // copy current GL state
prestate->diffusenodeid = elem->coinstate.diffusenodeid;
prestate->transpnodeid = elem->coinstate.transpnodeid;
elem->precachestateList.append(prestate);
elem->postcachestateList.append(poststate);
elem->precachestate()->cachebitmask = 0;
elem->postcachestate()->cachebitmask = 0;
elem->didsetbitmask = 0;
elem->didntsetbitmask = 0;
elem->cachebitmask = 0;
elem->opencacheflags = 0;
}
void
SoGLLazyElement::endCaching(SoState * state)
{
SoGLLazyElement * elem = getInstance(state);
*elem->postcachestate() = elem->glstate;
elem->postcachestate()->cachebitmask = elem->cachebitmask;
elem->precachestate()->cachebitmask = elem->didntsetbitmask;
// unset diffuse mask since it's used by the dependency test
elem->precachestate()->cachebitmask &= ~DIFFUSE_MASK;
// set diffuse mask if this cache depends on a material outside the
// cache.
if (elem->opencacheflags & FLAG_DIFFUSE_DEPENDENCY) {
elem->precachestate()->cachebitmask |= DIFFUSE_MASK;
}
elem->precachestateList.remove(elem->precachestateList.getLength()-1);
elem->postcachestateList.remove(elem->postcachestateList.getLength() - 1);
elem->opencacheflags = 0;
}
And using the method precachestate() instead of the variable in "SoGLLazyElement::send()"
I tested this solution, and it doesn't crash anymore.
Another even simpler solution could be for SoPrimitiveVertexCache constructor not to call SoGLLazyElement::beginCaching() if someone has already called it before. Though I have not tested this last one.
any traction on this?
I had a closer look onto this but I am not convinced the proposed solution is the right one. The SoPrimitiveVertexCache should IMHO not setup a new cache but rather use the existing. Will need to dive further into the code.
Softest of bumps
Hi Coin team,
under certain circumstances it can happen that a segmentation fault occurs in the function SoGLLazyElement::endCaching() at the line
because the postcachestate variable has been nullified in a previous call of SoGLLazyElement::endCaching().
The crash occurred in FreeCAD where some custom Inventor classes are involved but I was able to implement a little demo application using only Coin3D node types.
Here is the code of the demo:
So far the crash only seems to occur if all of the conditions below are fulfilled:
The steps to reproduce:
After the SoAnnotation has been added to the scene graph the functions SoGLLazyElement::beginCaching() and SoGLLazyElement::endCaching() will be called a few times but then there is a nested call of SoGLLazyElement::beginCaching() where the function getInstance() returns two times the same instance of SoGLLazyElement. Afterwards SoGLLazyElement::endCaching() will be called twice and inside the first call precachestate and postcachestate are nullified. In the second call postcachestate will be dereferenced and thus causes a segmentation fault because it's NULL.
For more details have a look at: https://forum.freecadweb.org/viewtopic.php?f=18&t=43305&start=10#p412537
Btw: The crash can be reproduced with the current master of Coin3D and SoQt. The crash can be reproduced on Windows and Linux and probably on any other OS