Skip to content

Commit 836e8b0

Browse files
committed
Fix RigTransformSoftware::prepareData
Instead of using copyFrom, explicitly copy pointers to required parts. Using copyFrom and then calling setVertexArray causes construction and deconstruction of BufferObject with some parts of it concurrently used by other threads in osg::GLBufferObject::compileBuffer which leads to use after free.
1 parent aa66653 commit 836e8b0

File tree

1 file changed

+48
-16
lines changed

1 file changed

+48
-16
lines changed

src/osgAnimation/RigTransformSoftware.cpp

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -101,35 +101,67 @@ void RigTransformSoftware::buildMinimumUpdateSet( const RigGeometry&rig )
101101
}
102102

103103

104-
bool RigTransformSoftware::prepareData(RigGeometry&rig)
104+
bool RigTransformSoftware::prepareData(RigGeometry& rig)
105105
{
106-
///set geom as it source
107-
if (rig.getSourceGeometry())
108-
rig.copyFrom(*rig.getSourceGeometry());
106+
if (rig.getSourceGeometry() == 0)
107+
return false;
108+
109+
osg::Geometry& source = *rig.getSourceGeometry();
109110

110-
osg::Vec3Array* normalSrc = dynamic_cast<osg::Vec3Array*>(rig.getSourceGeometry()->getNormalArray());
111-
osg::Vec3Array* positionSrc = dynamic_cast<osg::Vec3Array*>(rig.getSourceGeometry()->getVertexArray());
111+
osg::Vec3Array* const positionSrc = dynamic_cast<osg::Vec3Array*>(source.getVertexArray());
112112

113-
if(!(positionSrc) || positionSrc->empty() )
113+
if (positionSrc == 0 || positionSrc->empty())
114114
return false;
115-
if(normalSrc && normalSrc->size() != positionSrc->size())
115+
116+
osg::Vec3Array* const normalSrc = dynamic_cast<osg::Vec3Array*>(source.getNormalArray());
117+
118+
if (normalSrc != 0 && normalSrc->size() != positionSrc->size())
116119
return false;
117120

118121
/// setup Vertex and Normal arrays with copy of sources
119-
rig.setVertexArray(new osg::Vec3Array);
120-
osg::Vec3Array* positionDst = new osg::Vec3Array;
121-
rig.setVertexArray(positionDst);
122-
*positionDst = *positionSrc;
122+
osg::ref_ptr<osg::Vec3Array> positionDst(new osg::Vec3Array(*positionSrc, osg::CopyOp::SHALLOW_COPY));
123123
positionDst->setDataVariance(osg::Object::DYNAMIC);
124124

125-
if(normalSrc)
125+
if (osg::Array* const oldArray = rig.getVertexArray())
126+
if (osg::VertexBufferObject* const vbo = oldArray->getVertexBufferObject())
127+
positionDst->setVertexBufferObject(vbo);
128+
129+
rig.setVertexArray(positionDst);
130+
131+
if (normalSrc)
126132
{
127-
osg::Vec3Array* normalDst = new osg::Vec3Array;
128-
*normalDst = *normalSrc;
129-
rig.setNormalArray(normalDst, osg::Array::BIND_PER_VERTEX);
133+
osg::ref_ptr<osg::Vec3Array> normalDst(new osg::Vec3Array(*normalSrc, osg::CopyOp::SHALLOW_COPY));
130134
normalDst->setDataVariance(osg::Object::DYNAMIC);
135+
136+
if (osg::Array* const oldArray = rig.getNormalArray())
137+
if (osg::VertexBufferObject* const vbo = oldArray->getVertexBufferObject())
138+
normalDst->setVertexBufferObject(vbo);
139+
140+
rig.setNormalArray(normalDst, osg::Array::BIND_PER_VERTEX);
131141
}
132142

143+
rig.setStateSet(source.getStateSet());
144+
145+
rig.getPrimitiveSetList() = source.getPrimitiveSetList();
146+
147+
if (source.getColorArray())
148+
rig.setColorArray(source.getColorArray());
149+
150+
if (source.getSecondaryColorArray())
151+
rig.setSecondaryColorArray(source.getSecondaryColorArray());
152+
153+
if (source.getFogCoordArray())
154+
rig.setFogCoordArray(source.getFogCoordArray());
155+
156+
for (unsigned int ti = 0, n = source.getNumTexCoordArrays(); ti < n; ++ti)
157+
if (osg::Array* const array = source.getTexCoordArray(ti))
158+
rig.setTexCoordArray(ti, array);
159+
160+
osg::Geometry::ArrayList& arrayList = source.getVertexAttribArrayList();
161+
for (unsigned int vi = 0, n = arrayList.size(); vi < n; ++vi)
162+
if (osg::Array* array = arrayList[vi].get())
163+
rig.setVertexAttribArray(vi, array);
164+
133165
/// build minimal set of VertexGroup
134166
buildMinimumUpdateSet(rig);
135167

0 commit comments

Comments
 (0)