Bug / Crash IdentifyMovingColliders.Execute OutOfBound crash

[Guillaume]

Hi!

We recently had a crash in a obi job "IdentifyMovingColliders.Execute" (see attached picture to have the exact line).
We are using Obi Rope 6.4 but the code I'll talks about remains the same in 6.5

After few investigation, the crash seems to happen when for some reasons we loaded, then unloaded some scenes containing colliders without letting them do the UpdateIfNeeded on the tracker. By doing this, the shapes remaings zeroed (just allocated).

I wanted to check what was the initial cause of the bug by reading a bit more the sourcecode, and there is something I'm missing.

1. BurstColliderWorld.cellSpans will always grow to at least match the colliderCount (as seen in BurstColliderWorld.SetColliders)
   
2. When preparing the IdentifyMovingColliders parralel for job (inside BurstColliderWorld.UpdateWorld), we transfer ObiColliderWorld.colliderShapes into a NativeArray by doing so: 
   shapes = world.colliderShapes.AsNativeArray<BurstColliderShape>(cellSpans.count).
   We then have a shapes native array matching the cellSpans.count... but not world.colliderShapes.count
   
3. Then we schedule the parrallel for with cellSpans.count as parameter, so we are iterating on non initialized or removed shapes sometimes (since cellSpans.count >= colliderShapes.count)
   
4. Since we loaded/unloaded the shapes without them having time to UpdateIfNeeded, the shape.materialIndex stay at 0 (and the other parameters of the shape are zero-ed too!)
   
5. We then test shapes[i].materialIndex >= 0 which is true (since we are not using material, we should have put a -1 in the skipped UpdateIfNeeded)
   
6. Then we crash on the line velocityBounds.Expand(collisionMaterials[shapes[i].materialIndex].stickDistance); since collisionMaterials is empty (that make me think we should use materials for our ropes!)
   

I don't understand why we are basing the parrallel for on cellSpans.count rather than world.colliderShapes.count, because here this parrallel for will always access some "old shape data" and process them while it's not necessary.

But maybe I'm missing something here!

The code I would see in BurstColliderWorld.UpdateWorld:

Code:

            var identifyMoving = new IdentifyMovingColliders
            {
                movingColliders = movingColliders.AsParallelWriter(),
                shapes = world.colliderShapes.AsNativeArray<BurstColliderShape>(), // <--- Match the world.colliderShapes count
                // instead of: shapes = world.colliderShapes.AsNativeArray<BurstColliderShape>(cellSpans.count),
                rigidbodies = world.rigidbodies.AsNativeArray<BurstRigidbody>(),
                collisionMaterials = world.collisionMaterials.AsNativeArray<BurstCollisionMaterial>(),
                bounds = world.colliderAabbs.AsNativeArray<BurstAabb>(), // <--- same here
                // instead of: bounds = world.colliderAabbs.AsNativeArray<BurstAabb>(cellSpans.count),
                cellIndices = cellSpans.AsNativeArray<BurstCellSpan>(),
                colliderCount = colliderCount,
                dt = deltaTime
            };
            JobHandle movingHandle = identifyMoving.Schedule(colliderCount, 128); // <--- instead of (cellSpans.count, 128)

Then in the Execute code I think we will not go out of bounds since "i" is only used on:

1. bounds[i] which be the size of shapes 
   
2. shapes[i] since we matched the size of the initial shapes array
   
3. shapes[i].materialIndex >= 0 test will always be relevent since we are not iterating on a "ghost" shape that could have been added to the shapes, then removed before doing the UpdateIfNeeded thing (since UpdateIfNeeded is called in the ObiColliderWorld.GetInstance().UpdateColliders(); during the ObiUpdater.BeginStep)
   

Code:

        protected void BeginStep(float stepDeltaTime)
        {
            using (m_BeginStepPerfMarker.Auto())
            {
                // Update colliders right before collision detection:
                ObiColliderWorld.GetInstance().UpdateColliders(); // <----- This is where UpdateIfNeeded are call, where shapes are filled with correct values
                ObiColliderWorld.GetInstance().UpdateRigidbodies(solvers,stepDeltaTime);
                ObiColliderWorld.GetInstance().UpdateWorld(stepDeltaTime); // <------ This is where the IdentifyMovingColliders parrallel for takes place

Do you also think that this parrallel for is doing too much loop/work over time?
(maybe there are also other jobbed thing that do the same, but maybe I'm missing why)

Thanks!