From 1208bcc562e50b4bebfd397575577615a54cbda0 Mon Sep 17 00:00:00 2001 From: Jupeyy Date: Thu, 5 Apr 2018 04:17:21 +0200 Subject: [PATCH 1/2] Make mem_alloc, mem_free thread safe (fixes #1087) --- src/base/system.c | 47 +++++++++++++++++++++++++++++++---- src/base/system.h | 48 +++++++++++++++++++++++++++++++++--- src/engine/shared/engine.cpp | 2 ++ 3 files changed, 89 insertions(+), 8 deletions(-) diff --git a/src/base/system.c b/src/base/system.c index d6a1633d3..3f881dfaf 100644 --- a/src/base/system.c +++ b/src/base/system.c @@ -260,18 +260,36 @@ 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 */ + if(init_mem_lock == 0) + { + init_mem_lock = 1; + mem_lock = lock_create(); + } + if(mem_lock) + lock_wait(mem_lock); /* 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) + { + if(mem_lock) + lock_unlock(mem_lock); return NULL; + } tail = (struct MEMTAIL *)(((char*)(header+1))+size); header->size = size; header->filename = filename; @@ -289,14 +307,24 @@ void *mem_alloc_debug(const char *filename, int line, unsigned size, unsigned al 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) { + if(mem_lock) + lock_wait(mem_lock); MEMHEADER *header = (MEMHEADER *)p - 1; MEMTAIL *tail = (MEMTAIL *)(((char*)(header+1))+header->size); @@ -313,13 +341,17 @@ 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]; + if(mem_lock) + lock_wait(mem_lock); MEMHEADER *header = first; if(!file) file = io_open("memory.txt", IOFLAG_WRITE); @@ -336,9 +368,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) { memcpy(dest, source, size); @@ -356,6 +389,8 @@ void mem_zero(void *block,unsigned size) int mem_check_imp() { + if(mem_lock) + lock_wait(mem_lock); MEMHEADER *header = first; while(header) { @@ -367,6 +402,8 @@ int mem_check_imp() } header = header->next; } + if(mem_lock) + lock_unlock(mem_lock); return 1; } diff --git a/src/base/system.h b/src/base/system.h index a24c9b10e..713a70c85 100644 --- a/src/base/system.h +++ b/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: + +*/ +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: - + , */ 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: - + */ -void mem_free(void *block); +void mem_free_impl(void *block); + +/* + Function: mem_free + Frees a block allocated through . + + Remarks: + - Is safe on null pointers. + + See Also: + , +*/ +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 diff --git a/src/engine/shared/engine.cpp b/src/engine/shared/engine.cpp index 59d8db5a0..c083df5a8 100644 --- a/src/engine/shared/engine.cpp +++ b/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"); } From a83a89703e3cadf516c106a1a5cb12677e4718c9 Mon Sep 17 00:00:00 2001 From: Jupeyy Date: Thu, 5 Apr 2018 04:42:03 +0200 Subject: [PATCH 2/2] change position of mutex lock --- src/base/system.c | 51 ++++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/src/base/system.c b/src/base/system.c index 3f881dfaf..244562413 100644 --- a/src/base/system.c +++ b/src/base/system.c @@ -273,34 +273,33 @@ void *mem_alloc_impl(unsigned size, unsigned alignment) void *mem_alloc_debug(const char *filename, int line, unsigned size, unsigned alignment) { + /* TODO: add debugging */ + MEMTAIL *tail; + 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); + 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); - /* TODO: add debugging */ - MEMTAIL *tail; - MEMHEADER *header = (struct MEMHEADER *)mem_alloc_impl(size+sizeof(MEMHEADER)+sizeof(MEMTAIL), alignment); - dbg_assert(header != 0, "mem_alloc failure"); - if(!header) - { - if(mem_lock) - lock_unlock(mem_lock); - return NULL; - } - tail = (struct MEMTAIL *)(((char*)(header+1))+size); - header->size = size; - header->filename = filename; - header->line = line; 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) @@ -323,11 +322,12 @@ void mem_free_debug(void *p) { if(p) { - if(mem_lock) - lock_wait(mem_lock); 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); */ @@ -343,6 +343,7 @@ void mem_free_debug(void *p) if(mem_lock) lock_unlock(mem_lock); + mem_free_impl(header); } } @@ -350,9 +351,13 @@ void mem_free_debug(void *p) void mem_debug_dump(IOHANDLE file) { char buf[1024]; + MEMHEADER *header; + if(mem_lock) lock_wait(mem_lock); - MEMHEADER *header = first; + + header = first; + if(!file) file = io_open("memory.txt", IOFLAG_WRITE); @@ -368,6 +373,7 @@ void mem_debug_dump(IOHANDLE file) io_close(file); } + if(mem_lock) lock_unlock(mem_lock); } @@ -389,9 +395,13 @@ void mem_zero(void *block,unsigned size) int mem_check_imp() { + MEMHEADER *header; + if(mem_lock) lock_wait(mem_lock); - MEMHEADER *header = first; + + header = first; + while(header) { MEMTAIL *tail = (MEMTAIL *)(((char*)(header+1))+header->size); @@ -402,6 +412,7 @@ int mem_check_imp() } header = header->next; } + if(mem_lock) lock_unlock(mem_lock);