Merge pull request #1101 from Jupeyy/pr_mem_fix

Make mem_alloc, mem_free thread safe (fixes #1087)

src/base/system.c

@@ -260,46 +260,74 @@ typedef struct MEMTAIL
 	int guard;
 } MEMTAIL;
 
+static LOCK mem_lock = (LOCK)0x0;
+static char init_mem_lock = 0;
 static struct MEMHEADER *first = 0;
 static const int MEM_GUARD_VAL = 0xbaadc0de;
 
+void *mem_alloc_impl(unsigned size, unsigned alignment)
+{
+	/* TODO: remove alignment parameter */
+	return malloc(size);
+}
+
 void *mem_alloc_debug(const char *filename, int line, unsigned size, unsigned alignment)
 {
-	/* TODO: fix alignment */
 	/* TODO: add debugging */
 	MEMTAIL *tail;
-	MEMHEADER *header = (struct MEMHEADER *)malloc(size+sizeof(MEMHEADER)+sizeof(MEMTAIL));
+	MEMHEADER *header = (struct MEMHEADER *)mem_alloc_impl(size+sizeof(MEMHEADER)+sizeof(MEMTAIL), alignment);
+
 	dbg_assert(header != 0, "mem_alloc failure");
 	if(!header)
 		return NULL;
-	tail = (struct MEMTAIL *)(((char*)(header+1))+size);
+
+	tail = (struct MEMTAIL *)(((char*)(header + 1)) + size);
 	header->size = size;
 	header->filename = filename;
 	header->line = line;
+	tail->guard = MEM_GUARD_VAL;
+
+	if(init_mem_lock == 0)
+	{
+		init_mem_lock = 1;
+		mem_lock = lock_create();
+	}
+
+	if(mem_lock)
+		lock_wait(mem_lock);
 
 	memory_stats.allocated += header->size;
 	memory_stats.total_allocations++;
 	memory_stats.active_allocations++;
 
-	tail->guard = MEM_GUARD_VAL;
-
 	header->prev = (MEMHEADER *)0;
 	header->next = first;
 	if(first)
 		first->prev = header;
 	first = header;
 
+	if(mem_lock)
+		lock_unlock(mem_lock);
+
 	/*dbg_msg("mem", "++ %p", header+1); */
 	return header+1;
 }
 
-void mem_free(void *p)
+void mem_free_impl(void *p)
+{
+	free(p);
+}
+
+void mem_free_debug(void *p)
 {
 	if(p)
 	{
 		MEMHEADER *header = (MEMHEADER *)p - 1;
 		MEMTAIL *tail = (MEMTAIL *)(((char*)(header+1))+header->size);
 
+		if(mem_lock)
+			lock_wait(mem_lock);
+
 		if(tail->guard != MEM_GUARD_VAL)
 			dbg_msg("mem", "!! %p", p);
 		/* dbg_msg("mem", "-- %p", p); */
@@ -313,14 +341,23 @@ void mem_free(void *p)
 		if(header->next)
 			header->next->prev = header->prev;
 
-		free(header);
+		if(mem_lock)
+			lock_unlock(mem_lock);
+
+		mem_free_impl(header);
 	}
 }
 
 void mem_debug_dump(IOHANDLE file)
 {
 	char buf[1024];
-	MEMHEADER *header = first;
+	MEMHEADER *header;
+
+	if(mem_lock)
+		lock_wait(mem_lock);
+
+	header = first;
+
 	if(!file)
 		file = io_open("memory.txt", IOFLAG_WRITE);
 
@@ -336,8 +373,10 @@ void mem_debug_dump(IOHANDLE file)
 
 		io_close(file);
 	}
-}
 
+	if(mem_lock)
+		lock_unlock(mem_lock);
+}
 
 void mem_copy(void *dest, const void *source, unsigned size)
 {
@@ -356,7 +395,13 @@ void mem_zero(void *block,unsigned size)
 
 int mem_check_imp()
 {
-	MEMHEADER *header = first;
+	MEMHEADER *header;
+
+	if(mem_lock)
+		lock_wait(mem_lock);
+
+	header = first;
+
 	while(header)
 	{
 		MEMTAIL *tail = (MEMTAIL *)(((char*)(header+1))+header->size);
@@ -368,6 +413,9 @@ int mem_check_imp()
 		header = header->next;
 	}
 
+	if(mem_lock)
+		lock_unlock(mem_lock);
+
 	return 1;
 }
 

src/base/system.h

@@ -92,6 +92,27 @@ GNUC_ATTRIBUTE((format(printf, 2, 3)));
 
 /* Group: Memory */
 
+/*
+	Function: mem_alloc_impl
+		Allocates memory.
+
+	Parameters:
+		size - Size of the needed block.
+		alignment - Alignment for the block.
+
+	Returns:
+		Returns a pointer to the newly allocated block. Returns a
+		null pointer if the memory couldn't be allocated.
+
+	Remarks:
+		- Passing 0 to size will allocated the smallest amount possible
+		and return a unique pointer.
+
+	See Also:
+		<mem_free_impl>
+*/
+void *mem_alloc_impl(unsigned size, unsigned alignment);
+
 /*
 	Function: mem_alloc
 		Allocates memory.
@@ -109,10 +130,14 @@ GNUC_ATTRIBUTE((format(printf, 2, 3)));
 		and return a unique pointer.
 
 	See Also:
-		<mem_free>
+		<mem_free>, <mem_alloc_impl>
 */
 void *mem_alloc_debug(const char *filename, int line, unsigned size, unsigned alignment);
+#ifdef CONF_DEBUG
 #define mem_alloc(s,a) mem_alloc_debug(__FILE__, __LINE__, (s), (a))
+#else
+#define mem_alloc(s,a) mem_alloc_impl(s, a)
+#endif
 
 /*
 	Function: mem_free
@@ -122,9 +147,26 @@ void *mem_alloc_debug(const char *filename, int line, unsigned size, unsigned al
 		- Is safe on null pointers.
 
 	See Also:
-		<mem_alloc>
+		<mem_alloc_impl>
 */
-void mem_free(void *block);
+void mem_free_impl(void *block);
+
+/*
+	Function: mem_free
+		Frees a block allocated through <mem_alloc>.
+
+	Remarks:
+		- Is safe on null pointers.
+
+	See Also:
+		<mem_alloc>, <mem_free_impl>
+*/
+void mem_free_debug(void *block);
+#ifdef CONF_DEBUG
+#define mem_free(p) mem_free_debug(p)
+#else
+#define mem_free(p) mem_free_impl(p)
+#endif
 
 /*
 	Function: mem_copy

src/engine/shared/engine.cpp

@@ -97,7 +97,9 @@ public:
 		if(!m_pConsole || !m_pStorage)
 			return;
 
+#ifdef CONF_DEBUG
 		m_pConsole->Register("dbg_dumpmem", "", CFGFLAG_SERVER|CFGFLAG_CLIENT, Con_DbgDumpmem, this, "Dump the memory");
+#endif
 		m_pConsole->Register("dbg_lognetwork", "", CFGFLAG_SERVER|CFGFLAG_CLIENT, Con_DbgLognetwork, this, "Log the network");
 	}