Re: memory leak detection patch (usable for

2005-09-23 Thread Oliver Stieber

--- Detlef Riekenberg <[EMAIL PROTECTED]> wrote:

> Hi Mike
> 
> > This patch is written for MSI, but could be adapted to other code.  It 
> > keeps a linked list of allocated memory and removes free'd memory from 
> > the list.
> 
> Nice Hint.
> I will reuse that, while working on the Printing subsystem.
> 
> > so hopefully somebody else will find it useful too.
> 
> should we implement this for all with something like:
> 
> WINEHEAP_DEFINES(prefix)
> wineheap_alloc
> wineheap_realloc
> .
> .
> .
> wineheap_dump_allocated_memory

There's a possibility that WINEs heap management is going to be improved at 
some point by keeping
all small allocations together and caching them to improve performance and 
reduce fragmentation.
I'm not sure how much this is vaporware but it could be fairy well intergrated 
with some kind of
memory tracking system.

Oliver
> 
> 
> 
> 
> -- 
> By By ...
>   ... Detlef
> 
> 
> 
> 





___ 
How much free photo storage do you get? Store your holiday 
snaps for FREE with Yahoo! Photos http://uk.photos.yahoo.com




Re: memory leak detection patch (usable for

2005-09-23 Thread Detlef Riekenberg
Hi Mike

> This patch is written for MSI, but could be adapted to other code.  It 
> keeps a linked list of allocated memory and removes free'd memory from 
> the list.

Nice Hint.
I will reuse that, while working on the Printing subsystem.

> so hopefully somebody else will find it useful too.

should we implement this for all with something like:

WINEHEAP_DEFINES(prefix)
wineheap_alloc
wineheap_realloc
.
.
.
wineheap_dump_allocated_memory




-- 
By By ...
  ... Detlef





Re: memory leak detection patch

2005-09-22 Thread Mike McCormack


Andreas Mohr wrote:


I think that we should concentrate on making valgrind the default leak
detection method in Wine, however.

Reasons:
- it catches many, many leaks *without any reprogramming effort*
- it catches many, many other problems
- it has other tools which are very useful, too (cache profiling, ...)
- it's so much better than any other "clever hack" that people come up with
  in 2 hours


Valgrind is great, and I've used it before, but it's a little heavy duty 
for what I want at the moment.


The main disadvantages are:

- it slows down Wine and programs running Wine. (Office 2003 install 
already takes a minute or so)
- it doesn't differenciate between Wine leaking memory and a program 
running in Wine leaking memory

- it requires a patched version of Wine to run
- it isn't built into Wine (ie. I could ship this patch with minimal 
overhead, and have my users point out memory leaks)

- it has issues following forks, and being run from scripts

The patch is not meant to replace valgrind, just to provide another way 
of finding problems in Wine code.


Mike




Re: memory leak detection patch

2005-09-22 Thread Andreas Mohr
Hi,

On Thu, Sep 22, 2005 at 12:08:20AM +0900, Mike McCormack wrote:
> 
> Hi,
> 
> This patch is written for MSI, but could be adapted to other code.  It 
> keeps a linked list of allocated memory and removes free'd memory from 
> the list.
> 
> It requires that you change all the HeapAlloc's or mallocs in a dll to 
> msi_alloc (for example), but can detect unfree'd memory and invalid free's.
> 
> I have discovered quite a few problems in my own code with this patch, 
> so hopefully somebody else will find it useful too.

First, thanks for fixing so many leaks!

I think that we should concentrate on making valgrind the default leak
detection method in Wine, however.

Reasons:
- it catches many, many leaks *without any reprogramming effort*
- it catches many, many other problems
- it has other tools which are very useful, too (cache profiling, ...)
- it's so much better than any other "clever hack" that people come up with
  in 2 hours

Thus IMHO valgrind should work perfectly with Wine, and it should be the
first thing that pops up when people want to do things like this: IOW
it should be documented rather prominently which are some useful ways of
using it in combination with Wine.

OTOH we might want to put a safety net in place in case valgrind somehow
becomes impossible to use in combination with Wine, so maybe we should
investigate a second leak tracer, too.

Or am I completely mistaken here?

Andreas




memory leak detection patch

2005-09-21 Thread Mike McCormack


Hi,

This patch is written for MSI, but could be adapted to other code.  It 
keeps a linked list of allocated memory and removes free'd memory from 
the list.


It requires that you change all the HeapAlloc's or mallocs in a dll to 
msi_alloc (for example), but can detect unfree'd memory and invalid free's.


I have discovered quite a few problems in my own code with this patch, 
so hopefully somebody else will find it useful too.


Mike

Index: dlls/msi/msi.c
===
RCS file: /home/mike/src/wine-cvs/wine/dlls/msi/msi.c,v
retrieving revision 1.102
diff -u -p -r1.102 msi.c
--- dlls/msi/msi.c	20 Sep 2005 11:59:14 -	1.102
+++ dlls/msi/msi.c	21 Sep 2005 15:02:32 -
@@ -1273,6 +1273,31 @@ end:
 }
 
 
+struct list msi_memory_list;
+CRITICAL_SECTION msi_mem_cs;
+
+CRITICAL_SECTION msi_mem_cs;
+static CRITICAL_SECTION_DEBUG msi_mem_cs_debug =
+{
+0, 0, &msi_mem_cs,
+{ &msi_mem_cs_debug.ProcessLocksList, 
+  &msi_mem_cs_debug.ProcessLocksList },
+  0, 0, { (DWORD_PTR)(__FILE__ ": msi_mem_cs") }
+};
+CRITICAL_SECTION msi_mem_cs = { &msi_mem_cs_debug, -1, 0, 0, 0, 0 };
+
+static void msi_dump_allocated_memory(void)
+{
+struct msi_mem_list *mem;
+
+EnterCriticalSection( &msi_mem_cs );
+LIST_FOR_EACH_ENTRY( mem, &msi_memory_list, struct msi_mem_list, entry )
+ERR("unfreed memory %p line %d file %s\n", mem, mem->line, mem->file);
+LeaveCriticalSection( &msi_mem_cs );
+}
+
+extern void msi_close_handles(void);
+
 /**
  *	DllMain
  */
@@ -1281,11 +1306,13 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL, 
 switch(fdwReason)
 {
 case DLL_PROCESS_ATTACH:
+list_init( &msi_memory_list );
 msi_hInstance = hinstDLL;
 DisableThreadLibraryCalls(hinstDLL);
 msi_dialog_register_class();
 break;
 case DLL_PROCESS_DETACH:
+msi_dump_allocated_memory();
 msi_dialog_unregister_class();
 /* FIXME: Cleanup */
 break;
Index: dlls/msi/msipriv.h
===
RCS file: /home/mike/src/wine-cvs/wine/dlls/msi/msipriv.h,v
retrieving revision 1.96
diff -u -p -r1.96 msipriv.h
--- dlls/msi/msipriv.h	21 Sep 2005 10:55:23 -	1.96
+++ dlls/msi/msipriv.h	21 Sep 2005 15:02:32 -
@@ -31,6 +31,7 @@
 #include "objidl.h"
 #include "winnls.h"
 #include "wine/list.h"
+#include "wine/debug.h"
 
 #define MSI_DATASIZEMASK 0x00ff
 #define MSITYPE_VALID0x0100
@@ -418,66 +419,136 @@ extern DWORD gUIFilter;
 extern LPVOID gUIContext;
 extern WCHAR gszLogFile[MAX_PATH];
 
-/* memory allocation macro functions */
-static inline void *msi_alloc( size_t len )
-{
-return HeapAlloc( GetProcessHeap(), 0, len );
-}
+extern struct list msi_memory_list;
+extern CRITICAL_SECTION msi_mem_cs;
 
-static inline void *msi_alloc_zero( size_t len )
-{
-return HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, len );
-}
+#define MSI_MEM_MAGIC 0x04250919
 
-static inline void *msi_realloc( void *mem, size_t len )
+struct msi_mem_list {
+int magic;
+struct list entry;
+int line;
+const char *file;
+};
+
+#define MSI_MEMORY_DEBUG
+
+static inline void *__msi_alloc( int flags, size_t len, int line, const char *file )
 {
-return HeapReAlloc( GetProcessHeap(), 0, mem, len );
+#ifdef MSI_MEMORY_DEBUG
+struct msi_mem_list *elem;
+EnterCriticalSection( &msi_mem_cs );
+elem = HeapAlloc( GetProcessHeap(), flags, sizeof *elem + len );
+if (elem)
+{
+elem->magic = MSI_MEM_MAGIC;
+elem->line = line;
+elem->file = file;
+list_add_tail( &msi_memory_list, &elem->entry );
+elem++;
+}
+LeaveCriticalSection( &msi_mem_cs );
+return elem;
+#else
+return HeapAlloc( GetProcessHeap(), flags, len );
+#endif
 }
 
-static inline void *msi_realloc_zero( void *mem, size_t len )
+static inline void *__msi_realloc( void *mem, int flags, size_t len, int line, const char *file )
 {
-return HeapReAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, mem, len );
-}
+#ifdef MSI_MEMORY_DEBUG
+struct msi_mem_list *elem = mem;
+if (!elem--) return NULL;
 
-static inline BOOL msi_free( void *mem )
+if (elem->magic != MSI_MEM_MAGIC)
+{
+MESSAGE("msi: bad realloc %s # %d\n", file, line);
+return NULL;
+}
+EnterCriticalSection( &msi_mem_cs );
+list_remove( &elem->entry );
+elem = HeapReAlloc( GetProcessHeap(), flags, elem, sizeof *elem + len );
+if (elem)
+{
+list_add_tail( &msi_memory_list, &elem->entry );
+elem++;
+}
+LeaveCriticalSection( &msi_mem_cs );
+
+return elem;
+#else
+return HeapReAlloc( GetProcessHeap(), flags, mem, len );
+#endif
+}
+static inline BOOL __msi_free( void *mem, int line, const char *file )
 {
+#ifdef MSI_MEMORY_DEBUG
+struct msi_mem_list *elem = mem;
+if (!elem--) return TRUE;
+if (elem->magic