Crash in Gu::SinglePersistentContactManifold::reduceBatchContactsConvex

[msinilo]

We sometimes crash in Gu::SinglePersistentContactManifold::reduceBatchContactsConvex, line 1735:
chosen[index] = true;
Doesn't happen in-house so all I have is a crash dump, but AFAICT index is invalid (GU_MANIFOLD_INVALID_INDEX), so clearly out of bounds.
My best theory is that when we do reduceManifoldContactsInDifferentPatches it drops 'duplicate' contacts by decreasing mEndIndex but it does not modify "root" patch's mTotalSize.
That means if we start with total size over GU_SINGLE_MANIFOLD_CACHE_SIZE we might hit the contact reduction path in addBatchManifoldContactsConvex. From that, it's possible to hit reduceBatchContactsConvex with fewer than 6 'live' contacts.
In our case it seems to be 2, so 2 'live' contacts, total size = 7, the third pick starts at index=invalid, but it can't update it, since both 'slots' are chosen so it fails the if(!chosen[i]) test.

The fix seems to be 1 line in reduceManifoldContactsInDifferentPatches:
manifoldContacts[l] = manifoldContacts[nextPatch->mEndIndex-1];
nextPatch->mEndIndex--;

•            contactPatch[i]->mTotalSize--;
  

... but to be 100% honest I'm not quite sure if I fully understand what's going on so could use a second opinion

[PierreTerdiman]

We fixed something recently that could be exactly this crash. Could you try replacing reduceManifoldContactsInDifferentPatches with this updated version?

	PX_FORCE_INLINE PxU32 reduceManifoldContactsInDifferentPatches(PCMContactPatch** contactPatch, PxU32 numContactPatch, MeshPersistentContact* manifoldContacts, PxU32 numContacts, const aos::FloatVArg sqReplaceBreaking)	const
	{
		using namespace aos;

		for(PxU32 i=0; i<numContactPatch; ++i)
		{
			PCMContactPatch* rootPatch = contactPatch[i];
			//this make sure the patch is the root before we do the contact reduction, otherwise, we will do duplicate work
			if(rootPatch->mRoot == rootPatch)
			{
				PCMContactPatch* currentPatch = rootPatch;
				while(currentPatch)
				{
					PCMContactPatch* nextPatch = currentPatch->mNextPatch;
					if(nextPatch)
					{
						for(PxU32 k = currentPatch->mStartIndex; k<currentPatch->mEndIndex; ++k)
						{
							for(PxU32 l = nextPatch->mStartIndex; l < nextPatch->mEndIndex; ++l)
							{
								Vec3V dif = V3Sub(manifoldContacts[l].mLocalPointB, manifoldContacts[k].mLocalPointB);
								FloatV d = V3Dot(dif, dif);
								if(FAllGrtr(sqReplaceBreaking, d))
								{
									//if two manifold contacts are within threshold, we will get rid of the manifold contacts in the other contact patch
									manifoldContacts[l] = manifoldContacts[nextPatch->mEndIndex-1];
									nextPatch->mEndIndex--;
									//the root's mTotalSize is what decides whether this patch list still needs contact
									//reduction later on, so it has to stay in sync with the contacts actually left in the list.
									PX_ASSERT(rootPatch->mTotalSize>0);
									rootPatch->mTotalSize--;
									numContacts--;
									l--;
								}
							}
						}
					}
					currentPatch = nextPatch;
				}
			}
		}
	
		return numContacts;
	}

[msinilo]

Thanks for a quick response! Yeah that looks similar to what we tried, I'll use your version. It'll be a while before we can confirm it helped.