Batch Table 'instanceIndex is out of range' error when editing a polyline

[hpinkos]

Reported on the forum: https://groups.google.com/forum/?hl=en#!topic/cesium-dev/gbBjAyFs1JQ

I've seen this a number of times when editing polyline positions, but haven't found a reliable way to reproduce the crash. I asked the user from the forum if they had a code example to share.

[ghost]

I don't know if it helps or not but I've came across this problem.
What happens (at least in my case) I had a polyline added to the screen, its potions where calculated using callpack property while mouse moved and on left clicked I remove the polyline.

Seems that the callback tries to set the polylines position but it has already been removed.

Hope it helps.

[hpinkos]

Thanks for the info @matankastel!

[difnicolas]

Ran into problems with this issue quite often in recent changes to our code base. Adding a try catch around a block of code was able to mitigate the issue for us.
Source/Scene/PolylineCollection.js
Inside of the PolylineCollection.prototype.update Function.
Line 463 where the else statement is is where i put the try around. so it looks like:

            } else {
                try {
                    var length = polylinesToUpdate.length;

[matsoo]

Ran into this problem when running code that removed polylines while the HTML container used by the Cesium Viewer was hidden.

A quick look at the code indicates that the problem is in the makeDirty function in Polyline. It calls _updatePolyline which in turn stores the polyline in the internal array _polylinesToUpdate in PolylineCollection.

If you somehow trigger makeDirty and then remove the polyline before the render loop has a chance to run, the crash will occur the next time PolylineCollection.update runs. Update will then try to access the already removed polyline from the internal array.

As all set functions in Polyline will trigger makeDirty there is probably several ways to get this crash. A workaround for our particular scenario of removing polylines while the viewer is hidden is to stop the rendering loop for the Cesium viewer by setting viewer.useDefaultRenderLoop = false when hiding the HTML element. This also prevents the _polylinesToUpdate array from filling up with multiple versions of the same polyline, as _updatePolyline does not have a duplicate check and the render loop may add a new copy each tick.

[hpinkos]

Thanks for the details @difnicolas, @matsoo! That should make it easier for us to track down.
Or if you have time, we'd be happy to review a pull request with the fix!

[markerikson]

Can confirm that I'm seeing the exact same behavior in my application. Still trying to track down what we recently changed that exposed this bug for us, but the description matches what I'm seeing Cesium do.

[hpinkos]

Thanks @markerikson! If you figure out a way to reproduce this consistently, let me know. I haven't been able to do so, which has made it tricky to debug the problem.

[markerikson]

I can definitely reproduce it consistently, it just involves a large internal proprietary application :)

The sequence described earlier seems to be correct overall. If you modify a polyline object, Cesium marks it as dirty and adds it to the list of polylines to be updated for that collection. If you also remove it in that same synchronous execution sequence, Cesium decrements its count of active polyline instances right away, but when it gets around to later re-rendering asynchronously, the polyline is still in the list to be updated, and the polyline's index may be greater than the current instance count, causing the error to be thrown.

I'll see if I can maybe put together a Sandcastle example today that replicates the behavior.

[markerikson]

Purely off the top of my head, one potential fix might be to update PolylineCollection.prototype.remove like this:

        if (this.contains(polyline)) {
            this._polylines[polyline._index] = undefined; // Removed later
            
+           if(this._polylinesToUpdate.length && this._polylinesToUpdate.indexOf(polyline) >= 0) {
+               this._polylinesToUpdate = this._polylinesToUpdate.filter(function(updatedPolyline) {
+                   return updatedPolyline !== polyline;
+               });
+           }
            
            this._polylinesRemoved = true;
            this._createVertexArray = true;
            this._createBatchTable = true;
            if (defined(polyline._bucket)) {
                var bucket = polyline._bucket;
                bucket.shaderProgram = bucket.shaderProgram && bucket.shaderProgram.destroy();
                bucket.pickShaderProgram = bucket.pickShaderProgram && bucket.pickShaderProgram.destroy();
            }
            polyline._destroy();
            return true;
        }

        return false;
    };

Similarly, either the _updatePolyline() function or update() might want to do some filtering so that a given polyline is only added or updated once.

[markerikson]

I'm coming back around to look at this, since we're still occasionally seeing this affect our applications. The good news is I was just able to come up with a very trivial repro in the sandcastle:

var viewer = new Cesium.Viewer('cesiumContainer');

var polylines = new Cesium.PolylineCollection();

viewer.scene.primitives.add(polylines);

const firstCoords = [-75, 35, -125, 35];
const secondCoords = [-75, 37,-125, 37];
const thirdCoords = [-75, 37,-125, 35];


const firstLine = polylines.add({
    positions : Cesium.Cartesian3.fromDegreesArray(firstCoords),
    width : 3,
    material : Cesium.Material.fromType("Color", {color : Cesium.Color.RED})
});
            

const secondLine = polylines.add({
    positions : Cesium.Cartesian3.fromDegreesArray(secondCoords),
    width : 3,
    material : Cesium.Material.fromType("Color", {color : Cesium.Color.BLUE})
});


Sandcastle.addToolbarButton('Change Polylines', function() {
    firstLine.width = 4;
    secondLine.width = 5;
    polylines.remove(secondLine);
});

Clicking the button always causes a Cesium crash.

I'm going to try to put together a fix and submit a PR.

[markerikson]

I've confirmed that my small suggested fix works against the Sandcastle repro I just pasted in. It's trivial enough that a Cesium dev could easily just put that in, or I can submit a PR myself - whichever would get it into the codebase faster. My team is almost to the end of a long development cycle, and we'll be doing a final round of library upgrades before that. We'd really like to have this rolled into Cesium 1.45 in May so that we can pick up that change before we freeze our development.

[hpinkos]

@markerikson thanks for looking into this! If you can submit a PR that would be great. I should have time to review and merge it before the 1.45 release if you get it open in the next day or so.

[markerikson]

Working on it now. I haven't signed the CLA yet, but I'll do that momentarily.

[markerikson]

PR up, CLA sent.

[cesium-concierge]

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/forum/?hl=en#!topic/cesium-dev/gbBjAyFs1JQ

If this issue affects any of these threads, please post a comment like the following:

The issue at #4983 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.

---

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

[hpinkos]

@matankastel @difnicolas @matsoo thanks to @markerikson this is now fixed! The fix will be included in the Cesium 1.45 release available on May 1