highfidelity/hifi — pull request #3867 by AndrewMeadows (2014-11-26 18:48:36+00:00)
Title: don't lookup entity by EntityID when deleting script

I wast testing other stuff and suffered a crash. Looking into it I realized that it wasn't caused by any of the work I was doing and I decided to fix it in its own PR.

To reproduce the crash, run the gun.js script that then continually fire bullets at an oblique angle into the boundary of the domain... until it crashes. Might require a hundred bullets... be patient.

An excerpt of the callstack looks like this:

#3 0x00007ff92b2e7e14 in qt_assert_x(char const*, char const*, char const*, int) () from /usr/local/src/Qt/5.2.1/gcc_64/lib/libQt5Core.so.5
#4 0x00000000009e6db5 in QList::operator[] (this=0x34ab360, i=0) at /usr/local/src/Qt/5.2.1/gcc_64/include/QtCore/qlist.h:480
#5 0x0000000000bd7ec2 in EntityTreeElement::getEntityWithEntityItemID (this=0x13930030, id=...) at libraries/entities/src/EntityTreeElement.cpp:649
#6 0x0000000000ba627b in EntityTree::findEntityByEntityItemID (this=0x2a75c30, entityID=...) at libraries/entities/src/EntityTree.cpp:464
#7 0x00000000009dcbcd in EntityTreeRenderer::getPreviouslyLoadedEntityScript (this=0x7fff015a0d70, entityItemID=...) at interface/src/entities/EntityTreeRenderer.cpp:199
#8 0x00000000009e25c2 in EntityTreeRenderer::checkAndCallUnload (this=0x7fff015a0d70, entityID=...) at interface/src/entities/EntityTreeRenderer.cpp:883
#9 0x00000000009e2393 in EntityTreeRenderer::deletingEntity (this=0x7fff015a0d70, entityID=...) at interface/src/entities/EntityTreeRenderer.cpp:864
#10 0x00000000009ea6b8 in QtPrivate::FunctionPointer::call, void> (f=

The problem appears to be a race condition between multiple threads: one is currently deleting entities from the tree and another is deleting scripts while also performing EntityItem lookups... sometimes the first thread deletes entities out from under the second.

My fix is not to lock the EntityTree when deleting scripts, but to not bother doing the EntityItem lookup, which appears to be unnecessary (I submit that theory for review here).