search

Joshua-Ashton / VPhysics-Jolt

Volt (VPhysics Jolt) is a replacement physics module for the Source Engine.
MIT License
1.24k stars 67 forks source link

[GMOD] PostCollision / PhysCollisionScreenShake cause server crash #51

Closed Draiget closed 2 years ago

Draiget commented 2 years ago

Not sure it supposed to be working with Garry's Mod, but in case if anyone know how this could be fixed, I'll post some details of my issue here.

So I just created one cube and used stacker against it, once boxes fall and collide - crash is happened, I dive a bit deep into the problem and found that crash is caused by PhysCollisionScreenShake (game\server\physics.cpp) at pEvent->pInternalData->GetContactPoint( vecPos );. As you can see in the debugger, pInternalData of the event is an invalid pointer: UDP: Nvm, this is an VTable ptr, the data is seems to be at least non-nullptr:

image

post_collision_data

The call-stack is the following:

>   server.dll!__Z24PhysCollisionScreenShakeP21gamevcollisionevent_ti()    Unknown
    server.dll!__ZN11CBaseEntity17VPhysicsCollisionEiP21gamevcollisionevent_t()    Unknown
    server.dll!__ZN12CPhysicsProp17VPhysicsCollisionEiP21gamevcollisionevent_t()   Unknown
    server.dll!__ZN15CCollisionEvent13PostCollisionEP17vcollisionevent_t() Unknown
    vphysics.dll!`JoltPhysicsContactListener::FlushCallbacks'::`2'::<lambda_5>::operator()(JoltPhysicsContactListener::JoltPhysicsCollisionEvent & event) Line 356  C++
    vphysics.dll!JoltPhysicsContactListener::JoltPhysicsEventTracker<JoltPhysicsContactListener::JoltPhysicsCollisionEvent>::ForEach<1,`JoltPhysicsContactListener::FlushCallbacks'::`2'::<lambda_5>>(JoltPhysicsContactListener::FlushCallbacks::__l2::<lambda_5> func) Line 541   C++
    vphysics.dll!JoltPhysicsContactListener::FlushCallbacks() Line 360  C++
    vphysics.dll!JoltPhysicsEnvironment::Simulate(float deltaTime) Line 793 C++
    server.dll!__ZL9PhysFramef()   Unknown
    server.dll!__ZN12CPhysicsHook26FrameUpdatePostEntityThinkEv()  Unknown

Crashes in PhysCollisionScreenShake:

    57241A0E  comiss      xmm0,dword ptr ds:[57658AF0h]  
    57241A15  jbe         __Z24PhysCollisionScreenShakeP21gamevcollisionevent_ti+133h (57241A83h)  
    57241A17  mov         ecx,dword ptr [esi+1Ch]  
    57241A1A  lea         edx,[ebp-10h]  
    57241A1D  push        edx  
    57241A1E  mov         eax,dword ptr [ecx]  
>   57241A20  call        dword ptr [eax+4]  
    57241A23  movss       xmm0,dword ptr [esi+18h]  
    57241A28  mulss       xmm0,dword ptr [ebp-20h]  
    57241A2D  mov         eax,dword ptr ds:[5797B0D4h]  
    57241A32  push        0  
    57241A34  push        0  
    57241A36  sub         esp,10h  
    57241A39  mulss       xmm0,dword ptr [eax+2Ch]  
    57241A3E  mov         eax,dword ptr ds:[5797B164h]  
    57241A43  mulss       xmm0,dword ptr ds:[576DCE80h]

at 57241A20 (*(void (__thiscall **)(_DWORD, float *))(**(_DWORD **)(a4 + 28) + 4))(*(_DWORD *)(a4 + 28), vecPos); which is probably pEvent->pInternalData->GetContactPoint( vecPos );, so it's trying to call GetContactPoint against nullptr.

Any ideas?

P.S. Manual windows build of VPhysics-Jolt@347e78d using SSDK-2013@0d8dcee, VS2022 (target for module is x32-Debug).

Joshua-Ashton commented 2 years ago

Thanks, I think I know the cause.

Can't believe I didn't catch this. I am adding to m_Events[ uThreadId ].emplace_back( std::forward< T >( val )... ); here, and the pInternalData is set to a local member of that.

When the vector grows, pInternalData becomes invalidated as the JoltPhysicsCollisionEvent gets reallocated.

A custom copy + move constructor needs to be implemented into the JoltPhysicsCollisionEvent class to re-point m_Event.pInternalData to the right &m_Data of the newly copied class.

Joshua-Ashton commented 2 years ago

(I won't have time to implement this tonight, so feel free to try it out yourself + PR and get the free GitHub points, otherwise I will fix it tomorrow).

Joshua-Ashton commented 2 years ago

Actually, I'll do it now... I lied.

Draiget commented 2 years ago

That is fastest issue response I've seen, heh, thank you!

Btw, I had an another issue with -> https://github.com/Joshua-Ashton/VPhysics-Jolt/blob/347e78d122973dc5135bd5c16c734f4ef39e01a6/vphysics_jolt/vjolt_environment.cpp#L916 There are some STL/SDL (not remember) checks that asserting on assignment of m_CachedObjects[ i ] to smth, I did a small replacement locally to m_CachedObjects.insert(m_CachedObjects.begin() + i, reinterpret_cast<IPhysicsObject*>(pBody->GetUserData())); and it works like a charm.

Joshua-Ashton commented 2 years ago

Can you try https://github.com/Joshua-Ashton/VPhysics-Jolt/pull/52 and see if that fixes it for you?

Draiget commented 2 years ago

Sure, let me try.

Joshua-Ashton commented 2 years ago

There are some STL/SDL (not remember) checks that asserting on assignment of m_CachedObjects[ i ] to smth, I did a small replacement locally to m_CachedObjects.insert(m_CachedObjects.begin() + i, reinterpret_cast<IPhysicsObject*>(pBody->GetUserData())); and it works like a charm.

Just looked, it should be resize and not reserve haha. I'll do a PR for that too.

Joshua-Ashton commented 2 years ago

https://github.com/Joshua-Ashton/VPhysics-Jolt/pull/53 tada

Joshua-Ashton commented 2 years ago

Thanks for the clear issue, some ez wins before bed :-)

Draiget commented 2 years ago

Hehe, no problem, I can confirm that https://github.com/Joshua-Ashton/VPhysics-Jolt/pull/52 fixes my issue, no crash so far.