Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core/command_queue_mt.h: Fix crash/hang when buffer fills up #42315

Merged
merged 2 commits into from
Oct 15, 2020
Merged

core/command_queue_mt.h: Fix crash/hang when buffer fills up #42315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
48e8da4
core/command_queue_mt: Customizable size and tests
lyuma Sep 24, 2020
9f654b4
core/command_queue_mt: Fix crash/hang when buffer fills up
lyuma Sep 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion core/command_queue_mt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "command_queue_mt.h"

#include "core/os/os.h"
#include "core/project_settings.h"

void CommandQueueMT::lock() {
mutex.lock();
Expand Down Expand Up @@ -71,7 +72,7 @@ CommandQueueMT::SyncSemaphore *CommandQueueMT::_alloc_sync_sem() {

bool CommandQueueMT::dealloc_one() {
tryagain:
if (dealloc_ptr == write_ptr) {
if (dealloc_ptr == (write_ptr_and_epoch >> 1)) {
// The queue is empty
return false;
}
Expand All @@ -94,6 +95,10 @@ bool CommandQueueMT::dealloc_one() {
}

CommandQueueMT::CommandQueueMT(bool p_sync) {
command_mem_size = GLOBAL_DEF_RST("memory/limits/command_queue/multithreading_queue_size_kb", DEFAULT_COMMAND_MEM_SIZE_KB);
ProjectSettings::get_singleton()->set_custom_property_info("memory/limits/command_queue/multithreading_queue_size_kb", PropertyInfo(Variant::INT, "memory/limits/command_queue/multithreading_queue_size_kb", PROPERTY_HINT_RANGE, "1,4096,1,or_greater"));
command_mem_size *= 1024;
command_mem = (uint8_t *)memalloc(command_mem_size);
if (p_sync) {
sync = memnew(Semaphore);
}
Expand Down
36 changes: 25 additions & 11 deletions core/command_queue_mt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
uint32_t read_ptr = 0;
uint32_t write_ptr = 0;
uint8_t *command_mem = nullptr;
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];
Mutex mutex;
Semaphore *sync = nullptr;
Expand All @@ -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
Expand All @@ -362,7 +366,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
Expand All @@ -375,12 +379,17 @@ 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];
*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;
}
}
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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();
}
Expand Down
Loading