Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crashes when a Ragdoll points to a destroyed PhysicsSpace #1428

Closed
stephengold opened this issue Dec 30, 2024 · 4 comments
Closed

crashes when a Ragdoll points to a destroyed PhysicsSpace #1428

stephengold opened this issue Dec 30, 2024 · 4 comments

Comments

@stephengold
Copy link
Contributor

stephengold commented Dec 30, 2024

As I mentioned before, I'm creating Java bindings for JoltPhysics. Earlier this month I began experiencing various crashes:

  • mutex lock failed
  • assertion failures in Ragdoll.cpp line 543 and Array.h line 455
  • EXCEPTION_ACCESS_VIOLATION
  • SIGSEGV in 4 different locations

After much experimentation, I've narrowed down the issue. I believe it relates to the order in which destructors for RagDoll and PhysicsSystem are invoked.

As you're probably aware, Java programmers aren't accustomed to thinking about object destruction. When using my Java bindings, C++ objects like physics systems and ragdoll references are allocated on the heap, and their destructors typically get invoked by an asynchronous automatic cleanup thread. In this scenario, the Java application cannot control the order in which those destructors are invoked. It's entirely possible for PhysicsSystem::~PhysicsSystem to be invoked before Ragdoll::~Ragdoll. In that case, ~Ragdoll accesses a destroyed PhysicsSystem.

This scenario is unlikely in C++ applications, where typical coding practices ensure destruction of all ragdolls prior to destruction of the physics system(s) they point to.

I ran an experiment where I disabled my cleanup thread so I could specify the destruction sequence. When the sequence was:

	for (RagdollRef r : mRagdolls)
		r.close();
        mPhysicsSystem.close();

the application always completed successfully, but when the sequence was:

        mPhysicsSystem.close();
	for (RagdollRef r : mRagdolls)
		r.close();

the application crashed every time at Jolt/Core/Array.h:455: (inIdx < mSize).

It ought to be possible to modify the PhysicsSystem class to keep track of all ragdolls that point to the system so that ~PhysicsSystem can invalidate those pointers. Or perhaps you can devise a better solution.

@jrouwe
Copy link
Owner

jrouwe commented Dec 30, 2024

Unfortunately this is not something that can be fixed. The PhysicsSystem does not know about the existance of the Ragdoll so it cannot destroy it first. Ragdoll is a higher level concept that the physics system should not know about. In general Jolt tries to avoid links between objects as much as possible. This is one of the main reasons why you can do many things in a thread safe way.

@stephengold
Copy link
Contributor Author

I understand.
I have another idea: how about changing Ragdoll::RemoveFromPhysicsSystem() to null out the mSystem field?

@jrouwe
Copy link
Owner

jrouwe commented Dec 30, 2024

That's not a proper solution either. You can do:

  • AddToPhysicsSystem
  • RemoveFromPhysicsSystem
  • AddToPhysicsSystem
  • ...

This is quite common as a character may only need an active ragdoll during certain situations (e.g. hit responses), but you want to keep the ragdoll around because it's expensive to create.

I don't think this is going to be the only case where destruction order matters and I would suggest looking into a more generic system to handle this. I took a quick look at jolt-jni:

  • Could JoltPhysicsObject keep track of dependencies and prevent premature destruction? E.g. an object reference to the class that created it? This would contain the PhysicsSystem for Ragdolls.
  • Can the Cleaner be smarter about ordering? E.g. can you order by type or perhaps by some priority value before running the actual clean so that PhysicsSystem is always destroyed last?

My Java skills are pretty low so excuse me if my suggestions don't make sense.

@stephengold
Copy link
Contributor Author

you want to keep the ragdoll around because it's expensive to create.

That makes sense.

Could JoltPhysicsObject keep track of dependencies and prevent premature destruction?

I already track dependencies for C++ objects contained in other C++ objects, such as vectors. However, tracking doesn't help with this issue because it only prevents containers from being cleaned while their contents are strongly reachable. In this issue, neither the ragdoll nor the physics space is strongly reachable.

can you order by type or perhaps by some priority value before running the actual clean so that PhysicsSystem is always destroyed last?

I don't know of any way to do this. I'll ask around.

Repository owner locked and limited conversation to collaborators Dec 31, 2024
@jrouwe jrouwe converted this issue into discussion #1431 Dec 31, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants