Model: fixed two crash issues when changing avatars

* When the GeometryReader has the last ref to the GeometryResource ptr
  It needs to hold on to the reference until invokeMethod is completed.
  Otherwise, invokeMethod will call a method on a deleted object, leading
  to memory corruption or crashes.

* When the Model URL is changed, the clusterMatrices are invalided and the
  RenderItemsSets are cleared.  However, there still might be renderItems in
  the scene pending changes list that might refer to those RenderItems and their
  clusterMatrices.  We need to guard against this access to prevent reading from
  memory that was previously freed.

Both of these issues were uncovered using the [avatar-thrasher](https://gist.github.com/hyperlogic/d82a61d141df43d576428501a82c5ee6) test script.
This commit is contained in:
Anthony Thibault 2016-04-02 21:48:22 -07:00
parent d899d7d696
commit 865a77ae20
3 changed files with 25 additions and 9 deletions

View file

@ -118,10 +118,8 @@ void GeometryReader::run() {
}
QThread::currentThread()->setPriority(QThread::LowPriority);
// Ensure the resource is still being requested
auto resource = _resource.toStrongRef();
if (!resource) {
qCWarning(modelnetworking) << "Abandoning load of" << _url << "; could not get strong ref";
if (!_resource.data()) {
qCWarning(modelnetworking) << "Abandoning load of" << _url << "; resource was deleted";
return;
}
@ -146,14 +144,27 @@ void GeometryReader::run() {
throw QString("unsupported format");
}
QMetaObject::invokeMethod(resource.data(), "setGeometryDefinition",
Q_ARG(void*, fbxGeometry));
// Ensure the resource has not been deleted, and won't be while invoke method is in flight.
auto resource = _resource.toStrongRef();
if (!resource) {
qCWarning(modelnetworking) << "Abandoning load of" << _url << "; could not get strong ref";
} else {
QMetaObject::invokeMethod(resource.data(), "setGeometryDefinition", Qt::BlockingQueuedConnection, Q_ARG(void*, fbxGeometry));
}
} else {
throw QString("url is invalid");
}
} catch (const QString& error) {
qCDebug(modelnetworking) << "Error reading " << _url << ": " << error;
QMetaObject::invokeMethod(resource.data(), "finishedLoading", Q_ARG(bool, false));
auto resource = _resource.toStrongRef();
// Ensure the resoruce has not been deleted, and won't be while invoke method is in flight.
if (!resource) {
qCWarning(modelnetworking) << "Abandoning load of" << _url << "; could not get strong ref";
} else {
QMetaObject::invokeMethod(resource.data(), "finishedLoading", Qt::BlockingQueuedConnection, Q_ARG(bool, false));
}
}
QThread::currentThread()->setPriority(originalPriority);

View file

@ -159,11 +159,13 @@ void Model::updateRenderItems() {
Transform collisionMeshOffset;
collisionMeshOffset.postTranslate(self->_offset);
uint32_t deleteGeometryCounter = self->_deleteGeometryCounter;
render::PendingChanges pendingChanges;
foreach (auto itemID, self->_modelMeshRenderItems.keys()) {
pendingChanges.updateItem<ModelMeshPartPayload>(itemID, [modelTransform, modelMeshOffset](ModelMeshPartPayload& data) {
pendingChanges.updateItem<ModelMeshPartPayload>(itemID, [modelTransform, modelMeshOffset, deleteGeometryCounter](ModelMeshPartPayload& data) {
// Ensure the model geometry was not reset between frames
if (data._model->isLoaded()) {
if (data._model && data._model->isLoaded() && deleteGeometryCounter == data._model->_deleteGeometryCounter) {
// lazy update of cluster matrices used for rendering. We need to update them here, so we can correctly update the bounding box.
data._model->updateClusterMatrices(modelTransform.getTranslation(), modelTransform.getRotation());
@ -1146,6 +1148,7 @@ void Model::setBlendedVertices(int blendNumber, const std::weak_ptr<NetworkGeome
}
void Model::deleteGeometry() {
_deleteGeometryCounter++;
_blendedVertexBuffers.clear();
_meshStates.clear();
_rig->destroyAnimGraph();

View file

@ -394,6 +394,8 @@ protected:
friend class ModelMeshPartPayload;
RigPointer _rig;
uint32_t _deleteGeometryCounter { 0 };
};
Q_DECLARE_METATYPE(ModelPointer)