Skip to content

Commit ea069e4

Browse files
RandomShaperRandomShaper
committed
Fix race conditions in separate-thread physics
1 parent ff2730a commit ea069e4

4 files changed

Lines changed: 58 additions & 27 deletions

File tree

‎core/command_queue_mt.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ bool CommandQueueMT::dealloc_one() {
9494
return true;
9595
}
9696

97-
CommandQueueMT::CommandQueueMT(bool p_sync) {
97+
CommandQueueMT::CommandQueueMT(bool p_sync, bool (*p_is_bypass_enabled_for_thread)()) {
9898
read_ptr_and_epoch = 0;
9999
write_ptr_and_epoch = 0;
100100
dealloc_ptr = 0;
@@ -108,8 +108,10 @@ CommandQueueMT::CommandQueueMT(bool p_sync) {
108108
sync_sems[i].in_use = false;
109109
}
110110
if (p_sync) {
111+
is_bypass_enabled_for_thread = p_is_bypass_enabled_for_thread;
111112
sync = memnew(Semaphore);
112113
} else {
114+
DEV_ASSERT(!p_is_bypass_enabled_for_thread);
113115
sync = nullptr;
114116
}
115117
}

‎core/command_queue_mt.h‎

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -225,23 +225,31 @@
225225
#define CMD_TYPE(N) Command##N<T, M COMMA(N) COMMA_SEP_LIST(TYPE_ARG, N)>
226226
#define CMD_ASSIGN_PARAM(N) cmd->p##N = p##N
227227

228-
#define DECL_PUSH(N) \
229-
template <class T, class M COMMA(N) COMMA_SEP_LIST(TYPE_PARAM, N)> \
230-
void push(T *p_instance, M p_method COMMA(N) COMMA_SEP_LIST(PARAM, N)) { \
231-
CMD_TYPE(N) *cmd = allocate_and_lock<CMD_TYPE(N)>(); \
232-
cmd->instance = p_instance; \
233-
cmd->method = p_method; \
234-
SEMIC_SEP_LIST(CMD_ASSIGN_PARAM, N); \
235-
unlock(); \
236-
if (sync) \
237-
sync->post(); \
228+
#define DECL_PUSH(N) \
229+
template <class T, class M COMMA(N) COMMA_SEP_LIST(TYPE_PARAM, N)> \
230+
void push(T *p_instance, M p_method COMMA(N) COMMA_SEP_LIST(PARAM, N)) { \
231+
if (unlikely(is_bypass_enabled_for_thread && is_bypass_enabled_for_thread())) { \
232+
(p_instance->*p_method)(COMMA_SEP_LIST(ARG, N)); \
233+
return; \
234+
} \
235+
CMD_TYPE(N) *cmd = allocate_and_lock<CMD_TYPE(N)>(); \
236+
cmd->instance = p_instance; \
237+
cmd->method = p_method; \
238+
SEMIC_SEP_LIST(CMD_ASSIGN_PARAM, N); \
239+
unlock(); \
240+
if (sync) \
241+
sync->post(); \
238242
}
239243

240244
#define CMD_RET_TYPE(N) CommandRet##N<T, M, COMMA_SEP_LIST(TYPE_ARG, N) COMMA(N) R>
241245

242246
#define DECL_PUSH_AND_RET(N) \
243247
template <class T, class M, COMMA_SEP_LIST(TYPE_PARAM, N) COMMA(N) class R> \
244248
void push_and_ret(T *p_instance, M p_method, COMMA_SEP_LIST(PARAM, N) COMMA(N) R *r_ret) { \
249+
if (unlikely(is_bypass_enabled_for_thread && is_bypass_enabled_for_thread())) { \
250+
*r_ret = (p_instance->*p_method)(COMMA_SEP_LIST(ARG, N)); \
251+
return; \
252+
} \
245253
SyncSemaphore *ss = _alloc_sync_sem(); \
246254
CMD_RET_TYPE(N) *cmd = allocate_and_lock<CMD_RET_TYPE(N)>(); \
247255
cmd->instance = p_instance; \
@@ -258,20 +266,24 @@
258266

259267
#define CMD_SYNC_TYPE(N) CommandSync##N<T, M COMMA(N) COMMA_SEP_LIST(TYPE_ARG, N)>
260268

261-
#define DECL_PUSH_AND_SYNC(N) \
262-
template <class T, class M COMMA(N) COMMA_SEP_LIST(TYPE_PARAM, N)> \
263-
void push_and_sync(T *p_instance, M p_method COMMA(N) COMMA_SEP_LIST(PARAM, N)) { \
264-
SyncSemaphore *ss = _alloc_sync_sem(); \
265-
CMD_SYNC_TYPE(N) *cmd = allocate_and_lock<CMD_SYNC_TYPE(N)>(); \
266-
cmd->instance = p_instance; \
267-
cmd->method = p_method; \
268-
SEMIC_SEP_LIST(CMD_ASSIGN_PARAM, N); \
269-
cmd->sync_sem = ss; \
270-
unlock(); \
271-
if (sync) \
272-
sync->post(); \
273-
ss->sem.wait(); \
274-
ss->in_use = false; \
269+
#define DECL_PUSH_AND_SYNC(N) \
270+
template <class T, class M COMMA(N) COMMA_SEP_LIST(TYPE_PARAM, N)> \
271+
void push_and_sync(T *p_instance, M p_method COMMA(N) COMMA_SEP_LIST(PARAM, N)) { \
272+
if (unlikely(is_bypass_enabled_for_thread && is_bypass_enabled_for_thread())) { \
273+
(p_instance->*p_method)(COMMA_SEP_LIST(ARG, N)); \
274+
return; \
275+
} \
276+
SyncSemaphore *ss = _alloc_sync_sem(); \
277+
CMD_SYNC_TYPE(N) *cmd = allocate_and_lock<CMD_SYNC_TYPE(N)>(); \
278+
cmd->instance = p_instance; \
279+
cmd->method = p_method; \
280+
SEMIC_SEP_LIST(CMD_ASSIGN_PARAM, N); \
281+
cmd->sync_sem = ss; \
282+
unlock(); \
283+
if (sync) \
284+
sync->post(); \
285+
ss->sem.wait(); \
286+
ss->in_use = false; \
275287
}
276288

277289
#define MAX_CMD_PARAMS 13
@@ -322,6 +334,7 @@ class CommandQueueMT {
322334
SyncSemaphore sync_sems[SYNC_SEMAPHORES];
323335
Mutex mutex;
324336
Semaphore *sync;
337+
bool (*is_bypass_enabled_for_thread)();
325338

326339
template <class T>
327340
T *allocate() {
@@ -487,7 +500,7 @@ class CommandQueueMT {
487500
unlock();
488501
}
489502

490-
CommandQueueMT(bool p_sync);
503+
CommandQueueMT(bool p_sync, bool (*is_bypass_enabled_for_thread)() = nullptr);
491504
~CommandQueueMT();
492505
};
493506

‎servers/physics_2d/physics_2d_server_wrap_mt.cpp‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@
3232

3333
#include "core/os/os.h"
3434

35+
static thread_local bool cmd_queue_bypass_enabled = false;
36+
37+
static bool is_cmd_queue_bypass_enabled() {
38+
return cmd_queue_bypass_enabled;
39+
}
40+
3541
void Physics2DServerWrapMT::thread_exit() {
3642
exit.set();
3743
}
@@ -41,6 +47,10 @@ void Physics2DServerWrapMT::thread_step(real_t p_delta) {
4147
step_sem.post();
4248
}
4349

50+
void Physics2DServerWrapMT::thread_flush_and_halt() {
51+
thread_halted_semaphore.post();
52+
}
53+
4454
void Physics2DServerWrapMT::_thread_callback(void *_instance) {
4555
Physics2DServerWrapMT *vsmt = reinterpret_cast<Physics2DServerWrapMT *>(_instance);
4656

@@ -77,6 +87,8 @@ void Physics2DServerWrapMT::step(real_t p_step) {
7787

7888
void Physics2DServerWrapMT::sync() {
7989
if (create_thread) {
90+
command_queue.push(this, &Physics2DServerWrapMT::thread_flush_and_halt);
91+
thread_halted_semaphore.wait();
8092
if (first_frame) {
8193
first_frame = false;
8294
} else {
@@ -93,6 +105,7 @@ void Physics2DServerWrapMT::flush_queries() {
93105

94106
void Physics2DServerWrapMT::end_sync() {
95107
physics_2d_server->end_sync();
108+
cmd_queue_bypass_enabled = false;
96109
}
97110

98111
void Physics2DServerWrapMT::init() {
@@ -130,7 +143,7 @@ void Physics2DServerWrapMT::finish() {
130143
}
131144

132145
Physics2DServerWrapMT::Physics2DServerWrapMT(Physics2DServer *p_contained, bool p_create_thread) :
133-
command_queue(p_create_thread) {
146+
command_queue(p_create_thread, p_create_thread ? is_cmd_queue_bypass_enabled : (bool (*)()) nullptr) {
134147
physics_2d_server = p_contained;
135148
create_thread = p_create_thread;
136149

‎servers/physics_2d/physics_2d_server_wrap_mt.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ class Physics2DServerWrapMT : public Physics2DServer {
6161
Semaphore step_sem;
6262
void thread_step(real_t p_delta);
6363

64+
Semaphore thread_halted_semaphore;
65+
void thread_flush_and_halt();
66+
6467
void thread_exit();
6568

6669
bool first_frame;

0 commit comments

Comments
 (0)