TypeError: Cannot set property 'geometry' of undefined

[CHBaker]

I'm rendering Cesium with Angular-Cesium. But this problem also occured when just running Cesium.js, which is Why I'm also posting here.

I'm rendering a kml stream and updating it every 1.5 seconds. Sometimes, it loads perfectly, no errors, and sometimes I get the error below. this is with no code changes. Also, If I set the interval to 10 seconds, It seems to solve the problem as far as I can tell... But of course, I'm trying to get an interval of 1.5 seconds.

My code is really simple, here is my Angular code. Notice the AfterViewInit hook.

export class GlobeComponent implements OnInit, AfterViewInit {
    cesiumViewer;
    options;
    kml$;

    constructor(private viewersManager: MapsManagerService,
                private globeService: GlobeService) {}

    ngOnInit() {
      // console.log(this.cesiumViewer);
    }

    ngAfterViewInit() {
        const viewer = this.viewersManager.getMap('main-map').getCesiumViewer()

        this.options = {
            camera: viewer.scene.camera,
            canvas: viewer.scene.canvas
        };

        this.kml$ = new Cesium.KmlDataSource(this.options);

        viewer.dataSources.add(
        this.kml$
        );

        setInterval(() => {
            this.kml$.load('http://..../rest/kml/accept/all', this.options);
        }, 1500);

    }
}

[hpinkos]

@CHBaker Can you give us an example KML file that causes the error? Otherwise it's going to be really difficult for us to reproduce

[CHBaker]

@hpinkos the problem is it's a kml stream, when it was static it worked fine

[hpinkos]

@CHBaker Gotcha. It's likely an async problem. The previous load didn't finish by the time you called the next one, and some kind of race condition is happening. As a workaround, you could try waiting for the previous load to resolve before calling the next one:

function loadKml() {
    this.kml$.load('http://..../rest/kml/accept/all', this.options)
        .then(function() {
            setInterval(() => {
                loadKml();
            }, 1500);
        }
}

It would be really helpful for us to debug if you could share 2 sequential KML files that cause the crash. Thanks!

[CHBaker]

@hpinkos hmmm. I tried that, and no error would occur but the kml would not load. I'm sorry I do not have permission to share the kml. All I can say is that it plots lines on the globe :/

I could get it working with an inconvenient interval:

this.kml$ = new Cesium.KmlDataSource(this.options);

        viewer.dataSources.add(
            this.kml$.load('http://172.16.1.254:8080/rest/kml/accept/all', this.options)
        );

        setTimeout( () => {
            setInterval(() => {
                this.kml$.load('http://172.16.1.254:8080/rest/kml/accept/all', this.options);
            }, 2000);
        }, 5000)

after testing, it seems 5 seconds is the shortest time before the error occurs

[CHBaker]

Is there a better way to lad kml updates than setting an interval, maybe?

[hpinkos]

@mramato @tfili any ideas here?

[tfili]

KML has built in network link functionality. Have you tried loading a document that contains one instead of trying to reimplement it yourself? Something like this

<?xml version="1.0" encoding="UTF-8"?>
<NetworkLink id="link">
	<Link>
           <href>http://172.16.1.254:8080/rest/kml/accept/all</href>
           <refreshMode>onInterval</refreshMode>
           <refreshInterval>1.5</refreshInterval>
	</Link>
</NetworkLink>

[CHBaker]

hmm, @tfili no I've actually never seen network link functionality, I suppose I could give it a shot. Do I have to lay those within the cesium tags?

On the other hand is there some sort of hook I can wait for before running the initial source or the intervals? because even when I set a timeout of 10 seconds, it sometimes fails

[hpinkos]

Closing this since we don't have a way to reproduce the error. @CHBaker if this is still a problem for you feel free to reopen the issue.

[NichijouCC]

createGeometryResults.length is > the instances.length ,so alert error.in my debug

[hpinkos]

@NichijouCC do you have Sandcastle code example that reproduces the crash? I think this might be a geometry error, not an error in PrimitivePipeline

[arimbey]

I get this too #5947 (comment)

[shunter]

Note that this crash has nothing to do with KML or CZML, so much of the above commentary is a distraction.

Here is a Sandcastle that reproduces the crash using the entity layer directly:

Sandcastle

Now, before you get worked up about how I'm injecting code into Primitive.update, let me explain myself...

The root of the crash here is that we are creating geometry on the worker, but while the asynchronous creation is in flight, we delete one of the entities. Deleting the entity causes the batch to remove the geometry instance here:

https:/AnalyticalGraphicsInc/cesium/blob/3f3576443c700533a978ebd3e3e8c94f0fbb9179/Source/DataSources/StaticGeometryColorBatch.js#L84-L86

Because Primitive.geometryInstances is the same array as the batch is modifying, this delete takes effect immediately. Then, all geometry creation completes for the primitive, and it moves to the next step and asynchronously combines the createGeometryResults.

However, the lengths are mismatched - we have more createGeometryResults than geometryInstances at this point. pack does no validation on the data that it packs, so it ends up sending packed data that crashes unpack when it blindly assumes that geometries and instances are the same length here:

https:/AnalyticalGraphicsInc/cesium/blob/3f3576443c700533a978ebd3e3e8c94f0fbb9179/Source/Scene/PrimitivePipeline.js#L610-L625

So, to reproduce this in the Sandcastle, I need to make sure that the delete happens while it's in the CREATING state but before COMBINING starts. Patching Primitive.update is a reliable way to make sure it happens after the first time update is called.

[shunter]

I'm not sure which is the right fix. It seems dangerous for the batch to be directly manipulating the geometryInstances. I'm not sure from reading it whether the batch code was written with the awareness that the array is actually being shared or not. @mramato any recollections here?

Alternatively we could change packCombineGeometryParameters to just discard createGeometryResults for which we no longer have a geometryInstances. I guess they are correlated by index in the arrays. Maybe it needs to be smarter than that, since presumably adding new entities during CREATING could happen to, or we could delete then add and end up referencing the wrong geometry instance. This goes back to the danger of directly manipulating Primitive.geometryInstances externally from the batch, I guess.