From 48e8da4aac6c2f6c56f38e2778329178b27666e7 Mon Sep 17 00:00:00 2001 From: Lyuma Date: Thu, 24 Sep 2020 09:03:19 -0700 Subject: core/command_queue_mt: Customizable size and tests Adds unit tests for command_queue_mt.h/cpp In this revision, some unit tests will fail due to issue #42107. --- core/command_queue_mt.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'core/command_queue_mt.h') diff --git a/core/command_queue_mt.h b/core/command_queue_mt.h index d7a6a5bc43..7369c655ba 100644 --- a/core/command_queue_mt.h +++ b/core/command_queue_mt.h @@ -330,15 +330,15 @@ class CommandQueueMT { /***** BASE *******/ enum { - COMMAND_MEM_SIZE_KB = 256, - COMMAND_MEM_SIZE = COMMAND_MEM_SIZE_KB * 1024, + DEFAULT_COMMAND_MEM_SIZE_KB = 256, SYNC_SEMAPHORES = 8 }; - uint8_t *command_mem = (uint8_t *)memalloc(COMMAND_MEM_SIZE); + uint8_t *command_mem = nullptr; uint32_t read_ptr = 0; uint32_t write_ptr = 0; uint32_t dealloc_ptr = 0; + uint32_t command_mem_size = 0; SyncSemaphore sync_sems[SYNC_SEMAPHORES]; Mutex mutex; Semaphore *sync = nullptr; @@ -362,7 +362,7 @@ class CommandQueueMT { } else { // ahead of dealloc_ptr, check that there is room - if ((COMMAND_MEM_SIZE - write_ptr) < alloc_size + sizeof(uint32_t)) { + if ((command_mem_size - write_ptr) < alloc_size + sizeof(uint32_t)) { // no room at the end, wrap down; if (dealloc_ptr == 0) { // don't want write_ptr to become dealloc_ptr @@ -375,7 +375,7 @@ class CommandQueueMT { } // if this happens, it's a bug - ERR_FAIL_COND_V((COMMAND_MEM_SIZE - write_ptr) < 8, nullptr); + ERR_FAIL_COND_V((command_mem_size - write_ptr) < 8, nullptr); // zero means, wrap to beginning uint32_t *p = (uint32_t *)&command_mem[write_ptr]; -- cgit v1.2.3 From 9f654b441fffa613568e30a4c53a57390be69e12 Mon Sep 17 00:00:00 2001 From: Lyuma Date: Thu, 24 Sep 2020 09:55:38 -0700 Subject: 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. --- core/command_queue_mt.h | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) (limited to 'core/command_queue_mt.h') diff --git a/core/command_queue_mt.h b/core/command_queue_mt.h index 7369c655ba..0e5bc7f369 100644 --- a/core/command_queue_mt.h +++ b/core/command_queue_mt.h @@ -335,8 +335,8 @@ class CommandQueueMT { }; uint8_t *command_mem = nullptr; - uint32_t read_ptr = 0; - uint32_t write_ptr = 0; + uint32_t read_ptr_and_epoch = 0; + uint32_t write_ptr_and_epoch = 0; uint32_t dealloc_ptr = 0; uint32_t command_mem_size = 0; SyncSemaphore sync_sems[SYNC_SEMAPHORES]; @@ -348,7 +348,11 @@ class CommandQueueMT { // alloc size is size+T+safeguard 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: + uint32_t write_ptr = write_ptr_and_epoch >> 1; if (write_ptr < dealloc_ptr) { // behind dealloc_ptr, check that there is room @@ -379,8 +383,13 @@ class CommandQueueMT { // zero means, wrap to beginning uint32_t *p = (uint32_t *)&command_mem[write_ptr]; - *p = 0; - write_ptr = 0; + *p = 1; + 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; } } @@ -394,6 +403,7 @@ class CommandQueueMT { // allocate the command T *cmd = memnew_placement(&command_mem[write_ptr], T); write_ptr += size; + write_ptr_and_epoch = (write_ptr << 1) | (write_ptr_and_epoch & 1); return cmd; } @@ -419,19 +429,21 @@ class CommandQueueMT { tryagain: // tried to read an empty queue - if (read_ptr == write_ptr) { + if (read_ptr_and_epoch == write_ptr_and_epoch) { if (p_lock) { unlock(); } return false; } + uint32_t read_ptr = read_ptr_and_epoch >> 1; uint32_t size_ptr = read_ptr; uint32_t size = *(uint32_t *)&command_mem[read_ptr] >> 1; if (size == 0) { + *(uint32_t *)&command_mem[read_ptr] = 0; // clear in-use bit. //end of ringbuffer, wrap - read_ptr = 0; + read_ptr_and_epoch = 0 | (1 & ~read_ptr_and_epoch); // Invert epoch. goto tryagain; } @@ -441,6 +453,8 @@ class CommandQueueMT { read_ptr += size; + read_ptr_and_epoch = (read_ptr << 1) | (read_ptr_and_epoch & 1); + if (p_lock) { unlock(); } -- cgit v1.2.3