AnimDebugDraw: Prevent multi-threaded access to shared map collection.

It was possible for the render thread to iterate over a map while the main thread mutates the same map. This could have led to memory corruption and crashes. To fix this we make a copy of the collection and pass the copy to the lambda that executes on the render thread.

Anthony Thibault committed 8 years ago

Commit dc43d1b1 parent 5c78a462 files · +13 −7Browse files

Showing the changes to libraries/render-utils/src/AnimDebugDraw.cpp (1 of 2 files). Show all

Unified · Split

libraries/render-utils/src/AnimDebugDraw.cpp+11−6

@@ -149,11 +149,11 @@ void AnimDebugDraw::shutdown() { · 148 unmodified lines
149	149	}
150	150
151	151	void AnimDebugDraw::addAbsolutePoses(const std::string& key, AnimSkeleton::ConstPointer skeleton, const AnimPoseVec& poses, const AnimPose& rootPose, const glm::vec4& color) {
152		− _absolutePoses[key] = PosesInfo(skeleton, poses, rootPose, color);
	152	+ _posesInfoMap[key] = PosesInfo(skeleton, poses, rootPose, color);
153	153	}
154	154
155	155	void AnimDebugDraw::removeAbsolutePoses(const std::string& key) {
156		− _absolutePoses.erase(key);
	156	+ _posesInfoMap.erase(key);
157	157	}
158	158
159	159	static const uint32_t red = toRGBA(255, 0, 0, 255);
@@ -320,7 +320,12 @@ void AnimDebugDraw::update() { · 160 unmodified lines
320	320	return;
321	321	}
322	322	render::Transaction transaction;
323		− transaction.updateItem<AnimDebugDrawData>(_itemID, [&](AnimDebugDrawData& data) {
	323	+
	324	+ // Make a copy of the _posesInfoMap member variable, and pass the copy into the lambda.
	325	+ // This allows the body of the lambda, which executes on the render thread, to safely iterate over the map.
	326	+ std::shared_ptr<PosesInfoMap> posesInfoMapCopy;
	327	+ posesInfoMapCopy = std::make_shared<PosesInfoMap>(_posesInfoMap);
	328	+ transaction.updateItem<AnimDebugDrawData>(_itemID, [posesInfoMapCopy](AnimDebugDrawData& data) {
324	329
325	330	const size_t VERTICES_PER_BONE = (6 + (NUM_CIRCLE_SLICES * 2) * 3);
326	331	const size_t VERTICES_PER_LINK = 8 * 2;
@@ -332,7 +337,7 @@ void AnimDebugDraw::update() { · 5 unmodified lines
332	337	// figure out how many verts we will need.
333	338	int numVerts = 0;
334	339
335		− for (auto& iter : _absolutePoses) {
	340	+ for (auto& iter : *posesInfoMapCopy) {
336	341	AnimSkeleton::ConstPointer& skeleton = std::get<0>(iter.second);
337	342	numVerts += skeleton->getNumJoints() * VERTICES_PER_BONE;
338	343	for (auto i = 0; i < skeleton->getNumJoints(); i++) {
@@ -362,7 +367,7 @@ void AnimDebugDraw::update() { · 23 unmodified lines
362	367	}
363	368
364	369	// draw absolute poses
365		− for (auto& iter : _absolutePoses) {
	370	+ for (auto& iter : *posesInfoMapCopy) {
366	371	AnimSkeleton::ConstPointer& skeleton = std::get<0>(iter.second);
367	372	AnimPoseVec& absPoses = std::get<1>(iter.second);
368	373	AnimPose rootPose = std::get<2>(iter.second);
@@ -421,7 +426,7 @@ void AnimDebugDraw::update() { · 52 unmodified lines
421	426
422	427	data._indexBuffer->resize(sizeof(uint32_t) * numVerts);
423	428	for (int i = 0; i < numVerts; i++) {
424		− data._indexBuffer->setSubData<uint32_t>(i, (uint32_t)i);;
	429	+ data._indexBuffer->setSubData<uint32_t>(i, (uint32_t)i);
425	430	}
426	431	});
427	432	scene->enqueueTransaction(transaction);