phetsims / unit-rates

"Unit Rates" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 2 forks source link

TypeError: Cannot read property 'length' of null #200

Closed phet-steele closed 7 years ago

phet-steele commented 7 years ago
Uncaught TypeError: Cannot read property 'length' of null
    at ShoppingItemNode.getParent (Node.js?bust=1499874450676:777)
    at ShoppingItem.animationCompletedCallback (ShoppingItemDragHandler.js?bust=1499874450676:192)
    at ShoppingItem.step (URMovable.js?bust=1499874450676:122)
    at FruitScene.step (ShoppingScene.js?bust=1499874450676:299)
    at ShoppingCategory.step (ShoppingCategory.js?bust=1499874450676:57)
    at ShoppingModel.step (ShoppingModel.js?bust=1499874450676:92)
    at Sim.stepSimulation (Sim.js?bust=1499874450676:830)
    at Sim.runAnimationLoop (Sim.js?bust=1499874450676:770)
pixelzoom commented 7 years ago

I suspect that in ShoppingNode, visibleObserver has been queued to be called before dispose is called. And then when visibleObserver is called, it attempts to perform operations on self post-disposal. So the assertion in Node getParent (line 777) fails:

776   getParent: function() {
777      assert && assert( this._parents.length <= 1, 'Cannot call getParent on a node with multiple parents' );
778      return this._parents.length ? this._parents[ 0 ] : null;
779    },

@jonathanolson Can you confirm that _parents is scuttled when Node is disposed?

pixelzoom commented 7 years ago

Btw... I have been unable to reproduce this with ?ea&screens=1&fuzzMouse. And I'm unclear on why it would only happen infrequently.

jessegreenberg commented 7 years ago

Is this issue related to https://github.com/phetsims/scenery/issues/647 (SimpleDragHandler disposal)?

pixelzoom commented 7 years ago

Hmmm... could be.

jonathanolson commented 7 years ago

Can you confirm that _parents is scuttled when Node is disposed?

That's correct (checks for things like removeChild( disposedNode )).

Is this issue related to phetsims/scenery#647 (SimpleDragHandler disposal)?

I don't see an immediate connection. This assertion failure is stemming from a step() instead of a pointer event.

pixelzoom commented 7 years ago

Fixed in the above commit, using the same approach as in https://github.com/phetsims/graphing-lines/issues/83 (testing this.disposed). Tested using ?ea&fuzzMouse.

Since this is the second sim where I've had to do this, I'm a bit concerned that there is a general pattern to this problem. I will create a separate issue to discuss.

@phet-steele please verify.

pixelzoom commented 7 years ago

@phet-steele is still seeing this after pulling, took ~1 hour to hit in fuzz test.

https://github.com/phetsims/scenery/issues/649 is going to make this workaround unnecessary, so let's hold off on this until that's done.

pixelzoom commented 7 years ago

@phet-steele This has been fixed in phetsims/scenery#649, and the workaround in unit-rates has been reverted. Please pull scenery and unit-rates to verify.

phet-steele commented 7 years ago

Fuzzed master for an hour and saw no errors, closing.