Skip to content

Commit b16a8b8

Browse files
committed
CommandQueueMT: Fix race conditions
1 parent b79fe2e commit b16a8b8

File tree

1 file changed

+19
-5
lines changed

1 file changed

+19
-5
lines changed

core/templates/command_queue_mt.h

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
#include "core/typedefs.h"
4040

4141
class CommandQueueMT {
42+
static const size_t MAX_COMMAND_SIZE = 1024;
43+
4244
struct CommandBase {
4345
bool sync = false;
4446
virtual void call() = 0;
@@ -154,29 +156,38 @@ class CommandQueueMT {
154156
}
155157

156158
void _flush() {
159+
MutexLock lock(mutex);
160+
157161
if (unlikely(flush_read_ptr)) {
158162
// Re-entrant call.
159163
return;
160164
}
161165

162-
MutexLock lock(mutex);
166+
char cmd_backup[MAX_COMMAND_SIZE];
163167

164168
while (flush_read_ptr < command_mem.size()) {
165169
uint64_t size = *(uint64_t *)&command_mem[flush_read_ptr];
166-
flush_read_ptr += 8;
170+
flush_read_ptr += sizeof(uint64_t);
171+
167172
CommandBase *cmd = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
173+
174+
// Protect against race condition between this thread
175+
// during the call to the command and other threads potentially
176+
// invalidating the pointer due to reallocs.
177+
memcpy(cmd_backup, (char *)cmd, size);
178+
168179
uint32_t allowance_id = WorkerThreadPool::thread_enter_unlock_allowance_zone(lock);
169-
cmd->call();
180+
((CommandBase *)cmd_backup)->call();
170181
WorkerThreadPool::thread_exit_unlock_allowance_zone(allowance_id);
171182

172183
// Handle potential realloc due to the command and unlock allowance.
173184
cmd = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
174185

175186
if (unlikely(cmd->sync)) {
176187
sync_head++;
177-
lock.~MutexLock(); // Give an opportunity to awaiters right away.
188+
lock.temp_unlock(); // Give an opportunity to awaiters right away.
178189
sync_cond_var.notify_all();
179-
new (&lock) MutexLock(mutex);
190+
lock.temp_relock();
180191
// Handle potential realloc happened during unlock.
181192
cmd = reinterpret_cast<CommandBase *>(&command_mem[flush_read_ptr]);
182193
}
@@ -210,20 +221,23 @@ class CommandQueueMT {
210221
void push(T *p_instance, M p_method, Args &&...p_args) {
211222
// Standard command, no sync.
212223
using CommandType = Command<T, M, false, Args...>;
224+
static_assert(sizeof(CommandType) <= MAX_COMMAND_SIZE);
213225
_push_internal<CommandType, false>(p_instance, p_method, std::forward<Args>(p_args)...);
214226
}
215227

216228
template <typename T, typename M, typename... Args>
217229
void push_and_sync(T *p_instance, M p_method, Args... p_args) {
218230
// Standard command, sync.
219231
using CommandType = Command<T, M, true, Args...>;
232+
static_assert(sizeof(CommandType) <= MAX_COMMAND_SIZE);
220233
_push_internal<CommandType, true>(p_instance, p_method, std::forward<Args>(p_args)...);
221234
}
222235

223236
template <typename T, typename M, typename R, typename... Args>
224237
void push_and_ret(T *p_instance, M p_method, R *r_ret, Args... p_args) {
225238
// Command with return value, sync.
226239
using CommandType = CommandRet<T, M, R, Args...>;
240+
static_assert(sizeof(CommandType) <= MAX_COMMAND_SIZE);
227241
_push_internal<CommandType, true>(p_instance, p_method, r_ret, std::forward<Args>(p_args)...);
228242
}
229243

0 commit comments

Comments
 (0)