server: Avoid accessing free'd thread pointers.
eric.pouech at wanadoo.fr
Fri Nov 10 06:05:33 CST 2006
Mike McCormack wrote:
> Eric Pouech wrote:
>> well, the kill_thread in that case in only done when the waiting
>> thread also died while waiting (ie has been killed by some other way)
>> (in normal cases, the wait operation on the waiting side would just
>> return an error code)
>> the I'm not still conviced this path is actually executed in that case
> When the process is terminating because the user pressed ^C, the
> waiting thread will already be dead, so send_thread_wakeup will kill it.
maybe, depending on the order of events
>> what lead you to write the patch ?
> valgrind reported that wineserver accessed free'd memory.
I've seen it too, but it was only for current cursor not next, which
still means we have :
- the issue in process termination, where an iteration in the loop can
kill more than one item in the list
- the generic issue in the SAFE version, where we still access the
cursor after it has been potentially fixed
hence, the first topic is addressed by your patch, or the point below
> On IRC, Alexandre suggested the fix is to change kill_process() to
> keep killing the first thread in the list until there's no more threads.
the second topic can be fixed with something along those lines
diff --git a/include/wine/list.h b/include/wine/list.h
index 6ceef8c..82ceb82 100644
@@ -146,8 +146,8 @@ #define LIST_FOR_EACH(cursor,list) \
/* iterate through the list, with safety against removal */
#define LIST_FOR_EACH_SAFE(cursor, cursor2, list) \
for ((cursor) = (list)->next, (cursor2) = (cursor)->next; \
- (cursor) != (list); \
- (cursor) = (cursor2), (cursor2) = (cursor)->next)
+ (cursor) != (list) && ((cursor2) = (cursor)->next); \
+ (cursor) = (cursor2))
/* iterate through the list using a list entry */
#define LIST_FOR_EACH_ENTRY(elem, list, type, field) \
More information about the wine-devel