Allocator problem

[ForserX]

Double free

[vertver]

+1

[mtsamis]

Do you have a test case / example that can reproduce that?

[ForserX]

ForserX#6279

[vertver]

This reproducing if you're trying to destroy more than 5-10 objects per step/tick.

Destroy code:

void
physics::physics_body::destroy()
{
	destroyed = true;
	for (auto joint = body->GetJointList(); joint != nullptr; ) {
		const auto next_joint = joint->next;
		get_world().DestroyJoint(joint->joint);
		joint = next_joint;
	}
	
	get_world().DestroyBody(body);
	body = nullptr;
}

pre_tick() code:

        for (const auto body : scheduled_to_delete_bodies) {
		body->destroy();
		if (bodies.contains(body)) {
			bodies.erase(body);
		}
	}
	
	scheduled_to_delete_bodies.clear();

My guess is that the crash is because b2Contact and b2Fixture which is stored inside b2Body have been already cleared in another object, but the pointers have not been nullptr'ed (they are just 0xFDFDFDFD). Also, my suggestions, this body was already free somewhere before, and that is the reason why all properties marked as invalid by memory allocator.

[vertver]

this issue was fixed by this code

	stl::hash_set<b2Contact*> contacts;
	for (const auto body : scheduled_to_delete_bodies) {
		const auto phys_body = body->get_body();
		if (phys_body != nullptr) {
			// In this case, we're trying store contact pointer to schedule contact free
			for (int i = 0; i < phys_body->GetContactCount(); i++) {
				const auto contact = phys_body->GetContact(i);
				if (contact != nullptr) {
					contacts.insert(contact);
				}
			}

			// Second - try to delete all referenced links in body
			phys_body->ClearContacts();
		}
	}

	// Yep, this is most interesting part: we're deleting all founded by iterating
	// contacts and free map after processing
	for (const auto contact : contacts) {
		world_holder->GetContactManager().Destroy(contact);
	}

[mtsamis]

Yes this makes some sense; after having a look at it I think that the way b2World::RemoveDeadContacts works is incorrect when combined with how contact destruction logic. I believe the condition contact->m_flags & b2Contact::e_persistFlag can also access free'd memory in that case.

Thanks for providing a workaround to the issue, I'll try to reproduce this and explore other solutions as well when I have some time.

[RonYanDaik]

I have same issue.

[RonYanDaik]

stl::hash_set<b2Contact*> contacts;
	for (const auto body : scheduled_to_delete_bodies) {
		const auto phys_body = body->get_body();
		if (phys_body != nullptr) {
			// In this case, we're trying store contact pointer to schedule contact free
			for (int i = 0; i < phys_body->GetContactCount(); i++) {
				const auto contact = phys_body->GetContact(i);
				if (contact != nullptr) {
					contacts.insert(contact);
				}
			}

			// Second - try to delete all referenced links in body
			phys_body->ClearContacts();
		}
	}

	// Yep, this is most interesting part: we're deleting all founded by iterating
	// contacts and free map after processing
	for (const auto contact : contacts) {
		world_holder->GetContactManager().Destroy(contact);
	}

Where this code should be placed?

[ForserX]

Check this: https://github.com/ForserX/Asura2D/blob/master/src/engine/physics/physics_world.cpp