[Bug]: Possible race condition when paper in async mode

[alexandernst]

What happened?

I'm hitting a bug in which the paper is trying to sort elements that were recently removed from the graph.

The code in which I think the bug is located:

    sortViewsExact: function() {

        // Run insertion sort algorithm in order to efficiently sort DOM elements according to their
        // associated model `z` attribute.

        var $cells = $(this.cells).children('[model-id]');
        var cells = this.model.get('cells');

        sortElements($cells, function(a, b) {
            var cellA = cells.get(a.getAttribute('model-id'));
            var cellB = cells.get(b.getAttribute('model-id'));
            var zA = cellA.attributes.z || 0;                  <--- "cellA" will be undefined
            var zB = cellB.attributes.z || 0;
            return (zA === zB) ? 0 : (zA < zB) ? -1 : 1;
        });
    },

This is caused by the fact that $cells contains the elements that are still in the DOM, while cells contains the elements that are in the graph. When the paper is in async mode, a race condition might happen in such a way that a given element can be removed from the graph and this function can be triggered before the element has been removed form the DOM.

This will lead to the sortElements function trying to sort elements that don't exist.

Repro example: https://codesandbox.io/s/jointjs-possible-race-condition-embed-remove-link-tofront-pttdim

Proposed solution:

        var $cells = $(this.cells).children('[model-id]');
        var cells = this.model.get('cells');

+       var ids = cells.map(c => c.id);
+       $cells = $cells.filter((_, c) => ids.includes(c.getAttribute("model-id")))

        sortElements($cells, function(a, b) {

Version

3.5.5

What browsers are you seeing the problem on?

Chrome

What operating system are you seeing the problem on?

Mac

[kumilingus]

Please, don't use EXACT sorting in async mode. Use APPROX.

Similar to #1320. It's a won't-fix issue for now as the EXACT sorting is a candidate for removal.

[alexandernst]

The bug happens with APPROX as well (at least in my code base). Let me check if I can create another repro

[kumilingus]

sortViewsExact() is invoked only in EXACT sorting mode.

[alexandernst]

Ohhh.... I got to the bottom of the bug. Really interesting. The bug was actually being thrown by the ui.Navigator element, which wasn't set to APPROX (which means that it got the default value, which is EXACT). The Navigator is hooked to the same graph as my main paper, which means that all the events received by the paper itself are also received by the Navigator (with all the implications that this has, one of them being the fact that it would try to sort my elements).

This bug aside, I might have to reconsider using the Navigator as having it is basically equivalent to drawing everything twice, which might not be good for the performance. 🤔

[kumilingus]

Feel free to join this discussion 😃

[alexandernst]

This should be closed as it's an expected behavior (sort of).