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

This issue has been tracked since 2022-10-10.

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 wrote this answer on 2022-10-24

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 wrote this answer on 2022-10-24

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

kumilingus wrote this answer on 2022-10-24

sortViewsExact() is invoked only in EXACT sorting mode.

alexandernst wrote this answer on 2022-10-24

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 wrote this answer on 2022-10-24

Feel free to join this discussion 😃

alexandernst wrote this answer on 2022-10-24

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

More Details About Repo
Owner Name clientIO
Repo Name joint
Full Name clientIO/joint
Language JavaScript
Created Date 2009-09-11
Updated Date 2022-12-03
Star Count 3715
Watcher Count 155
Fork Count 817
Issue Count 51

YOU MAY BE INTERESTED

Issue Title Created Date Updated Date