From 9f654b441fffa613568e30a4c53a57390be69e12 Mon Sep 17 00:00:00 2001
From: Lyuma <xn.lyuma@gmail.com>
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.cpp |  2 +-
 core/command_queue_mt.h   | 26 ++++++++++++++++++++------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/core/command_queue_mt.cpp b/core/command_queue_mt.cpp
index 95fcd2c70e..a55eed5d3c 100644
--- a/core/command_queue_mt.cpp
+++ b/core/command_queue_mt.cpp
@@ -72,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;
 	}
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