core/command_queue_mt: Fix crash/hang when buffer fills up

This patch fixes two related issues. One is the race condition in issue #42107..
The other is a crash which happens when the reader is lapped near the end of the buffer.
This commit is contained in:
Lyuma 2020-09-24 09:55:38 -07:00
parent 48e8da4aac
commit 9f654b441f
2 changed files with 21 additions and 7 deletions

View File

@ -72,7 +72,7 @@ CommandQueueMT::SyncSemaphore *CommandQueueMT::_alloc_sync_sem() {
bool CommandQueueMT::dealloc_one() { bool CommandQueueMT::dealloc_one() {
tryagain: tryagain:
if (dealloc_ptr == write_ptr) { if (dealloc_ptr == (write_ptr_and_epoch >> 1)) {
// The queue is empty // The queue is empty
return false; return false;
} }

View File

@ -335,8 +335,8 @@ class CommandQueueMT {
}; };
uint8_t *command_mem = nullptr; uint8_t *command_mem = nullptr;
uint32_t read_ptr = 0; uint32_t read_ptr_and_epoch = 0;
uint32_t write_ptr = 0; uint32_t write_ptr_and_epoch = 0;
uint32_t dealloc_ptr = 0; uint32_t dealloc_ptr = 0;
uint32_t command_mem_size = 0; uint32_t command_mem_size = 0;
SyncSemaphore sync_sems[SYNC_SEMAPHORES]; SyncSemaphore sync_sems[SYNC_SEMAPHORES];
@ -348,7 +348,11 @@ class CommandQueueMT {
// alloc size is size+T+safeguard // alloc size is size+T+safeguard
uint32_t alloc_size = ((sizeof(T) + 8 - 1) & ~(8 - 1)) + 8; uint32_t alloc_size = ((sizeof(T) + 8 - 1) & ~(8 - 1)) + 8;
// Assert that the buffer is big enough to hold at least two messages.
ERR_FAIL_COND_V(alloc_size * 2 + sizeof(uint32_t) > command_mem_size, nullptr);
tryagain: tryagain:
uint32_t write_ptr = write_ptr_and_epoch >> 1;
if (write_ptr < dealloc_ptr) { if (write_ptr < dealloc_ptr) {
// behind dealloc_ptr, check that there is room // behind dealloc_ptr, check that there is room
@ -379,8 +383,13 @@ class CommandQueueMT {
// 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];
*p = 0; *p = 1;
write_ptr = 0; write_ptr_and_epoch = 0 | (1 & ~write_ptr_and_epoch); // Invert epoch.
// See if we can get the thread to run and clear up some more space while we wait.
// This is required if alloc_size * 2 + 4 > COMMAND_MEM_SIZE
if (sync) {
sync->post();
}
goto tryagain; goto tryagain;
} }
} }
@ -394,6 +403,7 @@ class CommandQueueMT {
// 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 += size; write_ptr += size;
write_ptr_and_epoch = (write_ptr << 1) | (write_ptr_and_epoch & 1);
return cmd; return cmd;
} }
@ -419,19 +429,21 @@ class CommandQueueMT {
tryagain: tryagain:
// tried to read an empty queue // tried to read an empty queue
if (read_ptr == write_ptr) { if (read_ptr_and_epoch == write_ptr_and_epoch) {
if (p_lock) { if (p_lock) {
unlock(); unlock();
} }
return false; return false;
} }
uint32_t read_ptr = read_ptr_and_epoch >> 1;
uint32_t size_ptr = read_ptr; uint32_t size_ptr = read_ptr;
uint32_t size = *(uint32_t *)&command_mem[read_ptr] >> 1; uint32_t size = *(uint32_t *)&command_mem[read_ptr] >> 1;
if (size == 0) { if (size == 0) {
*(uint32_t *)&command_mem[read_ptr] = 0; // clear in-use bit.
//end of ringbuffer, wrap //end of ringbuffer, wrap
read_ptr = 0; read_ptr_and_epoch = 0 | (1 & ~read_ptr_and_epoch); // Invert epoch.
goto tryagain; goto tryagain;
} }
@ -441,6 +453,8 @@ class CommandQueueMT {
read_ptr += size; read_ptr += size;
read_ptr_and_epoch = (read_ptr << 1) | (read_ptr_and_epoch & 1);
if (p_lock) { if (p_lock) {
unlock(); unlock();
} }