http://kegel.com/wine/valgrind/logs/ is updated as usual, this time with all of the log files I used to generate. For instance, http://kegel.com/wine/valgrind/logs/2009-10-22-19.57-summary.txt is a diff of just the first line of each error, to give you a quick idea what changed. (The logs are pretty noisy - if there's a crash that happens every other day, you'll see lots of churn from all the warnings that follow, or would follow, the crash.)
listview seems genuinely improved: http://kegel.com/wine/valgrind/logs/2009-10-22-19.57/vg-comctl32_listview.tx... is a lot shorter than http://kegel.com/wine/valgrind/logs/2009-10-21-19.42/vg-comctl32_listview.tx...
The script also shows a diff from the previous day for each test's log, e.g. http://kegel.com/wine/valgrind/logs/2009-10-22-19.57/vg-comctl32_listview-di...
Dan Kegel wrote:
http://kegel.com/wine/valgrind/logs/ is updated as usual, this time with all of the log files I used to generate. For instance, http://kegel.com/wine/valgrind/logs/2009-10-22-19.57-summary.txt is a diff of just the first line of each error, to give you a quick idea what changed. (The logs are pretty noisy - if there's a crash that happens every other day, you'll see lots of churn from all the warnings that follow, or would follow, the crash.)
listview seems genuinely improved: http://kegel.com/wine/valgrind/logs/2009-10-22-19.57/vg-comctl32_listview.tx... is a lot shorter than http://kegel.com/wine/valgrind/logs/2009-10-21-19.42/vg-comctl32_listview.tx...
The script also shows a diff from the previous day for each test's log, e.g. http://kegel.com/wine/valgrind/logs/2009-10-22-19.57/vg-comctl32_listview-di...
Looking at msvcrt.dll._fcvt leak reported in recent results it's not quite clear for me how could this happen. It uses heap block allocated as thread data and it should be freed on DLL_THREAD_DETACH. Is there any chance that this notification is skipped running inside winetest?
On Fri, Oct 23, 2009 at 9:19 AM, Nikolay Sivov bunglehead@gmail.com wrote:
Looking at msvcrt.dll._fcvt leak reported in recent results it's not quite clear for me how could this happen. It uses heap block allocated as thread data and it should be freed on DLL_THREAD_DETACH.
Really? I don't see it being freed. I see TlsFree being called, but I don't see the storage being freed...
Dan Kegel wrote:
On Fri, Oct 23, 2009 at 9:19 AM, Nikolay Sivov bunglehead@gmail.com wrote:
Looking at msvcrt.dll._fcvt leak reported in recent results it's not quite clear for me how could this happen. It uses heap block allocated as thread data and it should be freed on DLL_THREAD_DETACH.
Really? I don't see it being freed. I see TlsFree being called, but I don't see the storage being freed...
case DLL_THREAD_DETACH: /* Free TLS */ tls = TlsGetValue(msvcrt_tls_index); if (tls) { HeapFree(GetProcessHeap(),0,tls->efcvt_buffer); <= HeapFree(GetProcessHeap(),0,tls->asctime_buffer); HeapFree(GetProcessHeap(),0,tls->wasctime_buffer); HeapFree(GetProcessHeap(),0,tls->strerror_buffer); } HeapFree(GetProcessHeap(), 0, tls);
Did I miss something?
On Fri, Oct 23, 2009 at 9:37 AM, Nikolay Sivov bunglehead@gmail.com wrote:
Did I miss something?
Sorry, I'm not fully awake (and I think the last time I was fully awake was some time before I had children :-).
MSDN says "When a DLL is unloaded from a process as a result of an unsuccessful load of the DLL, termination of the process, or a call to FreeLibrary, the system does not call the DLL's entry-point function with the DLL_THREAD_DETACH value for the individual threads of the process. The DLL is only sent a DLL_PROCESS_DETACH notification. DLLs can take this opportunity to clean up all resources for all threads known to the DLL."
The DLL_PROCESS_DETACH handler might need to iterate over all things created by threads and free them. I think I've seen this before... check out urlmon/urlmon_main.c.
Dan Kegel wrote:
On Fri, Oct 23, 2009 at 9:37 AM, Nikolay Sivov bunglehead@gmail.com wrote:
Did I miss something?
Sorry, I'm not fully awake (and I think the last time I was fully awake was some time before I had children :-).
Same here I can tell you.
MSDN says "When a DLL is unloaded from a process as a result of an unsuccessful load of the DLL, termination of the process, or a call to FreeLibrary, the system does not call the DLL's entry-point function with the DLL_THREAD_DETACH value for the individual threads of the process. The DLL is only sent a DLL_PROCESS_DETACH notification. DLLs can take this opportunity to clean up all resources for all threads known to the DLL."
For process termination it's harmless, but for explicit unload it's a leak. Probably a similar thread data tracking should be added here too.
The DLL_PROCESS_DETACH handler might need to iterate over all things created by threads and free them. I think I've seen this before... check out urlmon/urlmon_main.c.
Thanks for info.
Nikolay Sivov bunglehead@gmail.com writes:
For process termination it's harmless, but for explicit unload it's a leak. Probably a similar thread data tracking should be added here too.
In theory yes, but in general that's not necessary. Particularly for something like msvcrt that won't get loaded and unloaded a lot of times in the same process.
On Fri, Oct 23, 2009 at 10:27 AM, Alexandre Julliard julliard@winehq.org wrote:
For process termination it's harmless, but for explicit unload it's a leak. Probably a similar thread data tracking should be added here too.
In theory yes, but in general that's not necessary. Particularly for something like msvcrt that won't get loaded and unloaded a lot of times in the same process.
Yeah, I have to say, I'm not especially worried about leaks on DLL unload of msvcrt or winex11.drv, and will probably file a bug for them (which we will leave open for a very long time), and add a suppression for those leaks. The name of the suppression will contain the bug number, so when people wonder what a suppression is for, they can just go look at the bug report.
That will reduce the noise in the valgrind logs and let us focus on things that matter more. - Dan
Dan Kegel wrote:
MSDN says "When a DLL is unloaded from a process as a result of an unsuccessful load of the DLL, termination of the process, or a call to FreeLibrary, the system does not call the DLL's entry-point function with the DLL_THREAD_DETACH value for the individual threads of the process. The DLL is only sent a DLL_PROCESS_DETACH notification. DLLs can take this opportunity to clean up all resources for all threads known to the DLL."
Forgot to say. This part of listview report looks like the same problem, but for winex11.drv
{ <insert_a_suppression_name_here> Memcheck:Leak fun:notify_alloc fun:RtlAllocateHeap fun:x11drv_init_thread_data fun:thread_init_display fun:X11DRV_create_win_data fun:X11DRV_WindowPosChanging fun:set_window_pos fun:USER_SetWindowPos fun:SetWindowPos fun:show_window fun:ShowWindow fun:WIN_CreateWindowEx fun:CreateWindowExA fun:create_parent_window fun:func_listview fun:run_test fun:main }
Dan Kegel wrote:
http://kegel.com/wine/valgrind/logs/ is updated as usual, this time with all of the log files I used to generate.
... listview seems genuinely improved: http://kegel.com/wine/valgrind/logs/2009-10-22-19.57/vg-comctl32_listview.tx... is a lot shorter than http://kegel.com/wine/valgrind/logs/2009-10-21-19.42/vg-comctl32_listview.tx...
I think a reason of EDIT_UnlockBuffer() warning found:
Invalid read of size 4 at EDIT_UnlockBuffer (edit.c:1219) by EditWndProc_common (edit.c:5393) by EditWndProcA (edit.c:5406) by ??? (library.h:159) by call_window_proc (winproc.c:469) by CallWindowProcA (winproc.c:2295) by CallWindowProcT (listview.c:1425) by EditLblWndProcT (listview.c:11279) by EditLblWndProcA (listview.c:11321) by ??? (library.h:159) by call_window_proc (winproc.c:469) by CallWindowProcA (winproc.c:2295) by editbox_subclass_proc (listview.c:576) by ??? (library.h:159) by call_window_proc (winproc.c:469) by WINPROC_CallProcWtoA (winproc.c:1279) by WINPROC_call_window (winproc.c:2216) by call_window_proc (message.c:1635) by send_message (message.c:2482) by SendMessageW (message.c:2605) Address 0x7f03fba4 is not stack'd, malloc'd or (recently) free'd
This is caused by destruction through WM_CLOSE that we send on Killfocus event through subclass procedure. The sequence looks like: 1) SetFocus(); 2) WM_KILLFOCUS at Edit wndpoc; 3) WM_COMMAND (EN_KILLFOCUS) from Edit to listview 4) WM_CLOSE from listview to edit 5) Edit destroyed in default procedure 6) returning from edit killfocus handler edit data EDITSTATE already freed, but es still contains pointer to its location.
Attached patch should fix this, looks like IsWindow is fast enough to call it every time but maybe somebody knows better way.
-
Alexandre Julliard -
Dan Kegel -
Nikolay Sivov