Jolt: Add null checks to jolt_free and jolt_aligned_free

[ryevdokimov]

Fixes getting an error: ERROR: Parameter "p_ptr" is null. at: Memory::free_static (core\os\memory.cpp:183) when creating new scene tabs while physics is enabled in the editor via PhysicsServer3D.set_active(true) by adding null checks to jolt_free() and jolt_aligned_free() in the jolt adapter layer.

I believe this happens during QuadTree optimizations when switching tabs. A nullptr will get passed to the free functions since physics bodies no longer exist.

@jrouwe, discussed with @mihe in RocketChat, and mentioned giving you a ping.

Stack Trace:

godot.windows.editor.dev.x86_64.exe!Memory::free_static(void * p_ptr, bool p_pad_align) Line 184 C++ godot.windows.editor.dev.x86_64.exe!jolt_free(void * p_mem) Line 56 C++ godot.windows.editor.dev.x86_64.exe!JPH::QuadTree::NodeID::operator delete[](void * inPointer) Line 33 C++ godot.windows.editor.dev.x86_64.exe!JPH::QuadTree::UpdatePrepare(const JPH::Array<JPH::Body *,JPH::STLAllocator<JPH::Body *>> & inBodies, JPH::Array<JPH::QuadTree::Tracking,JPH::STLAllocator<JPH::QuadTree::Tracking>> & ioTracking, JPH::QuadTree::UpdateState & outUpdateState, bool inFullRebuild) Line 391 C++ godot.windows.editor.dev.x86_64.exe!JPH::BroadPhaseQuadTree::Optimize() Line 88 C++ godot.windows.editor.dev.x86_64.exe!JPH::PhysicsSystem::OptimizeBroadPhase() Line 111 C++ godot.windows.editor.dev.x86_64.exe!JoltSpace3D::remove_object(const JPH::BodyID & p_jolt_id) Line 451 C++ godot.windows.editor.dev.x86_64.exe!JoltObject3D::_remove_from_space() Line 45 C++ godot.windows.editor.dev.x86_64.exe!JoltObject3D::set_space(JoltSpace3D * p_space) Line 91 C++ godot.windows.editor.dev.x86_64.exe!JoltPhysicsServer3D::body_set_space(RID p_body, RID p_space) Line 548 C++ godot.windows.editor.dev.x86_64.exe!CommandQueueMT::Command<PhysicsServer3D,void (__cdecl PhysicsServer3D::*)(RID,RID),0,RID &,RID &>::call_impl<0,1>(IndexSequence<0,1> __formal) Line 69 C++ godot.windows.editor.dev.x86_64.exe!CommandQueueMT::Command<PhysicsServer3D,void (__cdecl PhysicsServer3D::*)(RID,RID),0,RID &,RID &>::call() Line 62 C++ godot.windows.editor.dev.x86_64.exe!CommandQueueMT::_flush() Line 170 C++ godot.windows.editor.dev.x86_64.exe!CommandQueueMT::flush_all() Line 237 C++ godot.windows.editor.dev.x86_64.exe!PhysicsServer3DWrapMT::_thread_loop() Line 47 C++ godot.windows.editor.dev.x86_64.exe!call_with_variant_args_helper<PhysicsServer3DWrapMT>(PhysicsServer3DWrapMT * p_instance, void(PhysicsServer3DWrapMT::*)() p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<> __formal) Line 223 C++ godot.windows.editor.dev.x86_64.exe!call_with_variant_args<PhysicsServer3DWrapMT>(PhysicsServer3DWrapMT * p_instance, void(PhysicsServer3DWrapMT::*)() p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 337 C++ godot.windows.editor.dev.x86_64.exe!CallableCustomMethodPointer<PhysicsServer3DWrapMT,void>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 103 C++ godot.windows.editor.dev.x86_64.exe!Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 57 C++ godot.windows.editor.dev.x86_64.exe!Callable::call<>() Line 949 C++ godot.windows.editor.dev.x86_64.exe!WorkerThreadPool::_process_task(WorkerThreadPool::Task * p_task) Line 145 C++ godot.windows.editor.dev.x86_64.exe!WorkerThreadPool::_thread_function(void * p_user) Line 218 C++ godot.windows.editor.dev.x86_64.exe!Thread::callback(unsigned __int64 p_caller_id, const Thread::Settings & p_settings, void(*)(void *) p_callback, void * p_userdata) Line 64 C++ godot.windows.editor.dev.x86_64.exe!std::invoke<void (__cdecl*)(unsigned __int64,Thread::Settings const &,void (__cdecl*)(void *),void *),unsigned __int64,Thread::Settings,void (__cdecl*)(void *),void *>(void(*)(unsigned __int64, const Thread::Settings &, void(*)(void *), void *) && _Obj, unsigned __int64 && _Arg1, Thread::Settings && <_Args2_0>, void(*)(void *) && <_Args2_1>, void * && <_Args2_2>) Line 1680 C++ godot.windows.editor.dev.x86_64.exe!std::thread::_Invoke<std::tuple<void (__cdecl*)(unsigned __int64,Thread::Settings const &,void (__cdecl*)(void *),void *),unsigned __int64,Thread::Settings,void (__cdecl*)(void *),void *>,0,1,2,3,4>(void * _RawVals) Line 61 C++ godot.windows.editor.dev.x86_64.exe!thread_start<unsigned int (__cdecl*)(void *),1>(void * const parameter) Line 97 C++ kernel32.dll!00007ffe79e5e8d7() Unknown ntdll.dll!00007ffe7a1ec53c() Unknown

[mihe]

jrouwe, discussed with mihe in RocketChat, and mentioned giving you a ping.

I figured since a lot of allocators typically just no-op when freeing a nullptr (where Godot instead emits an error) Jolt might be making similar assumption about its allocator callbacks.

Looking at the code for JPH::QuadTree::UpdatePrepare it would seem like node_ids can in fact be nullptr if we have no bodies, so I'm guessing this is expected behavior?

[jrouwe]

Yes, Jolt indeed assumes that it can delete/free a nullptr

[jrouwe]

B.t.w. a reallocate should also accept a nullptr as old block.

[ryevdokimov]

Looks like it already does:

godot/modules/jolt_physics/jolt_globals.cpp

Lines 51 to 53 in 9d84f3d

	void *jolt_realloc(void *p_mem, size_t p_old_size, size_t p_new_size) {
	return Memory::realloc_static(p_mem, p_new_size);
	}

godot/core/os/memory.cpp

Lines 132 to 135 in 9d84f3d

	void *Memory::realloc_static(void *p_memory, size_t p_bytes, bool p_pad_align) {
	if (p_memory == nullptr) {
	return alloc_static(p_bytes, p_pad_align);
	}

[Repiteo]

Thanks!