Fix alignment and locking issues with CommandQueueMT
The allocations of commands in CommandQueueMT weren't aligned. This commit aligns all accesses on 64bit boundaries regardless of target platform. This ensures that all types are aligned. Lock-wise the semaphores were maked as usable when the command had ran but not when the synchronous stub had finished with it. This lead to a race condition where sometimes the semaphore got reused before it was waited on. We now mark the semaphore as free only once we're done waiting on it.
This commit is contained in:
parent
f5477ee36f
commit
24e7a54cd0
2 changed files with 14 additions and 9 deletions
|
@ -97,7 +97,7 @@ tryagain:
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
dealloc_ptr += (size >> 1) + sizeof(uint32_t);
|
dealloc_ptr += (size >> 1) + 8;
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -107,6 +107,7 @@ CommandQueueMT::CommandQueueMT(bool p_sync) {
|
||||||
write_ptr = 0;
|
write_ptr = 0;
|
||||||
dealloc_ptr = 0;
|
dealloc_ptr = 0;
|
||||||
mutex = Mutex::create();
|
mutex = Mutex::create();
|
||||||
|
command_mem = (uint8_t *)memalloc(COMMAND_MEM_SIZE);
|
||||||
|
|
||||||
for (int i = 0; i < SYNC_SEMAPHORES; i++) {
|
for (int i = 0; i < SYNC_SEMAPHORES; i++) {
|
||||||
|
|
||||||
|
@ -128,4 +129,5 @@ CommandQueueMT::~CommandQueueMT() {
|
||||||
|
|
||||||
memdelete(sync_sems[i].sem);
|
memdelete(sync_sems[i].sem);
|
||||||
}
|
}
|
||||||
|
memfree(command_mem);
|
||||||
}
|
}
|
||||||
|
|
|
@ -36,6 +36,7 @@
|
||||||
#include "core/os/semaphore.h"
|
#include "core/os/semaphore.h"
|
||||||
#include "core/simple_type.h"
|
#include "core/simple_type.h"
|
||||||
#include "core/typedefs.h"
|
#include "core/typedefs.h"
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@author Juan Linietsky <reduzio@gmail.com>
|
@author Juan Linietsky <reduzio@gmail.com>
|
||||||
*/
|
*/
|
||||||
|
@ -254,6 +255,7 @@
|
||||||
unlock(); \
|
unlock(); \
|
||||||
if (sync) sync->post(); \
|
if (sync) sync->post(); \
|
||||||
ss->sem->wait(); \
|
ss->sem->wait(); \
|
||||||
|
ss->in_use = false; \
|
||||||
}
|
}
|
||||||
|
|
||||||
#define CMD_SYNC_TYPE(N) CommandSync##N<T, M COMMA(N) COMMA_SEP_LIST(TYPE_ARG, N)>
|
#define CMD_SYNC_TYPE(N) CommandSync##N<T, M COMMA(N) COMMA_SEP_LIST(TYPE_ARG, N)>
|
||||||
|
@ -270,6 +272,7 @@
|
||||||
unlock(); \
|
unlock(); \
|
||||||
if (sync) sync->post(); \
|
if (sync) sync->post(); \
|
||||||
ss->sem->wait(); \
|
ss->sem->wait(); \
|
||||||
|
ss->in_use = false; \
|
||||||
}
|
}
|
||||||
|
|
||||||
#define MAX_CMD_PARAMS 13
|
#define MAX_CMD_PARAMS 13
|
||||||
|
@ -295,7 +298,6 @@ class CommandQueueMT {
|
||||||
|
|
||||||
virtual void post() {
|
virtual void post() {
|
||||||
sync_sem->sem->post();
|
sync_sem->sem->post();
|
||||||
sync_sem->in_use = false;
|
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -318,7 +320,7 @@ class CommandQueueMT {
|
||||||
SYNC_SEMAPHORES = 8
|
SYNC_SEMAPHORES = 8
|
||||||
};
|
};
|
||||||
|
|
||||||
uint8_t command_mem[COMMAND_MEM_SIZE];
|
uint8_t *command_mem;
|
||||||
uint32_t read_ptr;
|
uint32_t read_ptr;
|
||||||
uint32_t write_ptr;
|
uint32_t write_ptr;
|
||||||
uint32_t dealloc_ptr;
|
uint32_t dealloc_ptr;
|
||||||
|
@ -330,7 +332,7 @@ class CommandQueueMT {
|
||||||
T *allocate() {
|
T *allocate() {
|
||||||
|
|
||||||
// alloc size is size+T+safeguard
|
// alloc size is size+T+safeguard
|
||||||
uint32_t alloc_size = sizeof(T) + sizeof(uint32_t);
|
uint32_t alloc_size = ((sizeof(T) + 8 - 1) & ~(8 - 1)) + 8;
|
||||||
|
|
||||||
tryagain:
|
tryagain:
|
||||||
|
|
||||||
|
@ -360,7 +362,7 @@ class CommandQueueMT {
|
||||||
}
|
}
|
||||||
|
|
||||||
// if this happens, it's a bug
|
// if this happens, it's a bug
|
||||||
ERR_FAIL_COND_V((COMMAND_MEM_SIZE - write_ptr) < sizeof(uint32_t), NULL);
|
ERR_FAIL_COND_V((COMMAND_MEM_SIZE - write_ptr) < 8, NULL);
|
||||||
// zero means, wrap to beginning
|
// zero means, wrap to beginning
|
||||||
|
|
||||||
uint32_t *p = (uint32_t *)&command_mem[write_ptr];
|
uint32_t *p = (uint32_t *)&command_mem[write_ptr];
|
||||||
|
@ -372,12 +374,13 @@ class CommandQueueMT {
|
||||||
// Allocate the size and the 'in use' bit.
|
// Allocate the size and the 'in use' bit.
|
||||||
// First bit used to mark if command is still in use (1)
|
// First bit used to mark if command is still in use (1)
|
||||||
// or if it has been destroyed and can be deallocated (0).
|
// or if it has been destroyed and can be deallocated (0).
|
||||||
|
uint32_t size = (sizeof(T) + 8 - 1) & ~(8 - 1);
|
||||||
uint32_t *p = (uint32_t *)&command_mem[write_ptr];
|
uint32_t *p = (uint32_t *)&command_mem[write_ptr];
|
||||||
*p = (sizeof(T) << 1) | 1;
|
*p = (size << 1) | 1;
|
||||||
write_ptr += sizeof(uint32_t);
|
write_ptr += 8;
|
||||||
// allocate the command
|
// allocate the command
|
||||||
T *cmd = memnew_placement(&command_mem[write_ptr], T);
|
T *cmd = memnew_placement(&command_mem[write_ptr], T);
|
||||||
write_ptr += sizeof(T);
|
write_ptr += size;
|
||||||
return cmd;
|
return cmd;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -415,7 +418,7 @@ class CommandQueueMT {
|
||||||
goto tryagain;
|
goto tryagain;
|
||||||
}
|
}
|
||||||
|
|
||||||
read_ptr += sizeof(uint32_t);
|
read_ptr += 8;
|
||||||
|
|
||||||
CommandBase *cmd = reinterpret_cast<CommandBase *>(&command_mem[read_ptr]);
|
CommandBase *cmd = reinterpret_cast<CommandBase *>(&command_mem[read_ptr]);
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue