Skip to content

Commit 3135a40

Browse files
committed
Merge pull request #113525 from brycehutchings/fix_nonpod_memcpy
Fix crash in `command_queue_mt` due to destruction of wrong object
2 parents 8981ced + cb4d1b7 commit 3135a40

File tree

1 file changed

+11
-16
lines changed

1 file changed

+11
-16
lines changed

core/templates/command_queue_mt.h

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class CommandQueueMT {
6060
_FORCE_INLINE_ Command(T *p_instance, M p_method, FwdArgs &&...p_args) :
6161
CommandBase(NeedsSync), instance(p_instance), method(p_method), args(std::forward<FwdArgs>(p_args)...) {}
6262

63-
void call() {
63+
void call() override {
6464
call_impl(BuildIndexSequence<sizeof...(Args)>{});
6565
}
6666

@@ -128,7 +128,7 @@ class CommandQueueMT {
128128
command_mem.resize(size + alloc_size + sizeof(uint64_t));
129129
*(uint64_t *)&command_mem[size] = alloc_size;
130130
void *cmd = &command_mem[size + sizeof(uint64_t)];
131-
new (cmd) T(std::forward<Args>(p_args)...);
131+
memnew_placement(cmd, T(std::forward<Args>(p_args)...));
132132
pending.store(true);
133133
}
134134

@@ -164,44 +164,39 @@ class CommandQueueMT {
164164
return;
165165
}
166166

167-
char cmd_backup[MAX_COMMAND_SIZE];
167+
alignas(uint64_t) char cmd_local_mem[MAX_COMMAND_SIZE];
168168

169169
while (flush_read_ptr < command_mem.size()) {
170170
uint64_t size = *(uint64_t *)&command_mem[flush_read_ptr];
171171
flush_read_ptr += sizeof(uint64_t);
172172

173-
CommandBase *cmd = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
174-
175173
// Protect against race condition between this thread
176174
// during the call to the command and other threads potentially
177-
// invalidating the pointer due to reallocs.
178-
memcpy(cmd_backup, (char *)cmd, size);
175+
// invalidating the pointer due to reallocs by relocating the object.
176+
CommandBase *cmd_original = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
177+
CommandBase *cmd_local = reinterpret_cast<CommandBase *>(cmd_local_mem);
178+
memcpy(cmd_local_mem, (char *)cmd_original, size);
179179

180180
if (unique_flusher) {
181181
// A single thread will pump; the lock is only needed for the command queue itself.
182182
lock.temp_unlock();
183-
((CommandBase *)cmd_backup)->call();
183+
cmd_local->call();
184184
lock.temp_relock();
185185
} else {
186186
// At least we can unlock during WTP operations.
187187
uint32_t allowance_id = WorkerThreadPool::thread_enter_unlock_allowance_zone(lock);
188-
((CommandBase *)cmd_backup)->call();
188+
cmd_local->call();
189189
WorkerThreadPool::thread_exit_unlock_allowance_zone(allowance_id);
190190
}
191191

192-
// Handle potential realloc due to the command and unlock allowance.
193-
cmd = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
194-
195-
if (unlikely(cmd->sync)) {
192+
if (unlikely(cmd_local->sync)) {
196193
sync_head++;
197194
lock.temp_unlock(); // Give an opportunity to awaiters right away.
198195
sync_cond_var.notify_all();
199196
lock.temp_relock();
200-
// Handle potential realloc happened during unlock.
201-
cmd = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
202197
}
203198

204-
cmd->~CommandBase();
199+
cmd_local->~CommandBase();
205200

206201
flush_read_ptr += size;
207202
}

0 commit comments

Comments
 (0)