From 24e7a54cd0643ef9bfe2c22bc9099bba8f81f584 Mon Sep 17 00:00:00 2001
From: Hein-Pieter van Braam <hp@tmm.cx>
Date: Sat, 16 Feb 2019 03:28:02 +0100
Subject: [PATCH] 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.
---
 core/command_queue_mt.cpp |  4 +++-
 core/command_queue_mt.h   | 19 +++++++++++--------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/core/command_queue_mt.cpp b/core/command_queue_mt.cpp
index 36d4b0a32f..2bdf02295c 100644
--- a/core/command_queue_mt.cpp
+++ b/core/command_queue_mt.cpp
@@ -97,7 +97,7 @@ tryagain:
 		return false;
 	}
 
-	dealloc_ptr += (size >> 1) + sizeof(uint32_t);
+	dealloc_ptr += (size >> 1) + 8;
 	return true;
 }
 
@@ -107,6 +107,7 @@ CommandQueueMT::CommandQueueMT(bool p_sync) {
 	write_ptr = 0;
 	dealloc_ptr = 0;
 	mutex = Mutex::create();
+	command_mem = (uint8_t *)memalloc(COMMAND_MEM_SIZE);
 
 	for (int i = 0; i < SYNC_SEMAPHORES; i++) {
 
@@ -128,4 +129,5 @@ CommandQueueMT::~CommandQueueMT() {
 
 		memdelete(sync_sems[i].sem);
 	}
+	memfree(command_mem);
 }
diff --git a/core/command_queue_mt.h b/core/command_queue_mt.h
index 3fd660a3db..59eabd8786 100644
--- a/core/command_queue_mt.h
+++ b/core/command_queue_mt.h
@@ -36,6 +36,7 @@
 #include "core/os/semaphore.h"
 #include "core/simple_type.h"
 #include "core/typedefs.h"
+
 /**
 	@author Juan Linietsky <reduzio@gmail.com>
 */
@@ -254,6 +255,7 @@
 		unlock();                                                                              \
 		if (sync) sync->post();                                                                \
 		ss->sem->wait();                                                                       \
+		ss->in_use = false;                                                                    \
 	}
 
 #define CMD_SYNC_TYPE(N) CommandSync##N<T, M COMMA(N) COMMA_SEP_LIST(TYPE_ARG, N)>
@@ -270,6 +272,7 @@
 		unlock();                                                                     \
 		if (sync) sync->post();                                                       \
 		ss->sem->wait();                                                              \
+		ss->in_use = false;                                                           \
 	}
 
 #define MAX_CMD_PARAMS 13
@@ -295,7 +298,6 @@ class CommandQueueMT {
 
 		virtual void post() {
 			sync_sem->sem->post();
-			sync_sem->in_use = false;
 		}
 	};
 
@@ -318,7 +320,7 @@ class CommandQueueMT {
 		SYNC_SEMAPHORES = 8
 	};
 
-	uint8_t command_mem[COMMAND_MEM_SIZE];
+	uint8_t *command_mem;
 	uint32_t read_ptr;
 	uint32_t write_ptr;
 	uint32_t dealloc_ptr;
@@ -330,7 +332,7 @@ class CommandQueueMT {
 	T *allocate() {
 
 		// 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:
 
@@ -360,7 +362,7 @@ class CommandQueueMT {
 				}
 
 				// 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
 
 				uint32_t *p = (uint32_t *)&command_mem[write_ptr];
@@ -372,12 +374,13 @@ class CommandQueueMT {
 		// Allocate the size and the 'in use' bit.
 		// First bit used to mark if command is still in use (1)
 		// 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];
-		*p = (sizeof(T) << 1) | 1;
-		write_ptr += sizeof(uint32_t);
+		*p = (size << 1) | 1;
+		write_ptr += 8;
 		// allocate the command
 		T *cmd = memnew_placement(&command_mem[write_ptr], T);
-		write_ptr += sizeof(T);
+		write_ptr += size;
 		return cmd;
 	}
 
@@ -415,7 +418,7 @@ class CommandQueueMT {
 			goto tryagain;
 		}
 
-		read_ptr += sizeof(uint32_t);
+		read_ptr += 8;
 
 		CommandBase *cmd = reinterpret_cast<CommandBase *>(&command_mem[read_ptr]);