Stowage/godot
2
0
Fork 0
mirror of https://github.com/godotengine/godot.git synced 2025-12-07 13:49:54 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #113525 from brycehutchings/fix_nonpod_memcpy

Browse source 
Fix crash in `command_queue_mt` due to destruction of wrong object
This commit is contained in:
Rémi Verschelde 2025-12-05 12:55:53 +01:00
commit 3135a40699
No known key found for this signature in database
GPG key ID: C3336907360768E1
1 changed files with 11 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

27
core/templates/command_queue_mt.h
View file

@ -60,7 +60,7 @@ class CommandQueueMT {
_FORCE_INLINE_ Command(T *p_instance, M p_method, FwdArgs &&...p_args) : _FORCE_INLINE_ Command(T *p_instance, M p_method, FwdArgs &&...p_args) :
CommandBase(NeedsSync), instance(p_instance), method(p_method), args(std::forward<FwdArgs>(p_args)...) {} CommandBase(NeedsSync), instance(p_instance), method(p_method), args(std::forward<FwdArgs>(p_args)...) {}
void call() { void call() override {
call_impl(BuildIndexSequence<sizeof...(Args)>{}); call_impl(BuildIndexSequence<sizeof...(Args)>{});
} }
@ -128,7 +128,7 @@ class CommandQueueMT {
command_mem.resize(size + alloc_size + sizeof(uint64_t)); command_mem.resize(size + alloc_size + sizeof(uint64_t));
*(uint64_t *)&command_mem[size] = alloc_size; *(uint64_t *)&command_mem[size] = alloc_size;
void *cmd = &command_mem[size + sizeof(uint64_t)]; void *cmd = &command_mem[size + sizeof(uint64_t)];
new (cmd) T(std::forward<Args>(p_args)...); memnew_placement(cmd, T(std::forward<Args>(p_args)...));
pending.store(true); pending.store(true);
} }
@ -164,44 +164,39 @@ class CommandQueueMT {
return; return;
} }
char cmd_backup[MAX_COMMAND_SIZE]; alignas(uint64_t) char cmd_local_mem[MAX_COMMAND_SIZE];
while (flush_read_ptr < command_mem.size()) { while (flush_read_ptr < command_mem.size()) {
uint64_t size = *(uint64_t *)&command_mem[flush_read_ptr]; uint64_t size = *(uint64_t *)&command_mem[flush_read_ptr];
flush_read_ptr += sizeof(uint64_t); flush_read_ptr += sizeof(uint64_t);
CommandBase *cmd = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
// Protect against race condition between this thread // Protect against race condition between this thread
// during the call to the command and other threads potentially // during the call to the command and other threads potentially
// invalidating the pointer due to reallocs. // invalidating the pointer due to reallocs by relocating the object.
memcpy(cmd_backup, (char *)cmd, size); CommandBase *cmd_original = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
CommandBase *cmd_local = reinterpret_cast<CommandBase *>(cmd_local_mem);
memcpy(cmd_local_mem, (char *)cmd_original, size);
if (unique_flusher) { if (unique_flusher) {
// A single thread will pump; the lock is only needed for the command queue itself. // A single thread will pump; the lock is only needed for the command queue itself.
lock.temp_unlock(); lock.temp_unlock();
((CommandBase *)cmd_backup)->call(); cmd_local->call();
lock.temp_relock(); lock.temp_relock();
} else { } else {
// At least we can unlock during WTP operations. // At least we can unlock during WTP operations.
uint32_t allowance_id = WorkerThreadPool::thread_enter_unlock_allowance_zone(lock); uint32_t allowance_id = WorkerThreadPool::thread_enter_unlock_allowance_zone(lock);
((CommandBase *)cmd_backup)->call(); cmd_local->call();
WorkerThreadPool::thread_exit_unlock_allowance_zone(allowance_id); WorkerThreadPool::thread_exit_unlock_allowance_zone(allowance_id);
} }
// Handle potential realloc due to the command and unlock allowance. if (unlikely(cmd_local->sync)) {
cmd = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
if (unlikely(cmd->sync)) {
sync_head++; sync_head++;
lock.temp_unlock(); // Give an opportunity to awaiters right away. lock.temp_unlock(); // Give an opportunity to awaiters right away.
sync_cond_var.notify_all(); sync_cond_var.notify_all();
lock.temp_relock(); lock.temp_relock();
// Handle potential realloc happened during unlock.
cmd = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
} }
cmd->~CommandBase(); cmd_local->~CommandBase();
flush_read_ptr += size; flush_read_ptr += size;
} }