systray[3/4]: Better validate icon owner
Robert Shearman
rob at codeweavers.com
Thu Feb 7 04:46:00 CST 2008
Kirill K. Smirnov wrote:
>> I don't get why you need this change. PostMessage should correctly
>> handle the case where icon->owner has been destroyed and adding a call
>> to IsWindow just introduces a race condition.
>>
>
> Just a testcase:
> 1) run any wine app (to be sure that explorer is running), e.g winecfg
> 2) run taskmgr
> 3) select taskmgr process in tasklist and kill it. taskmgr window will
> dissapear but tray icon not.
> 4) move mouse cursor over taskmgr tray icon. Before this patch it will not
> dissapear, but should.
>
This is pretty much the same test I did when I originally wrote the code.
> The reason is that ERROR_INVALID_HANDLE is not the only value which indicates
> the absence of owner window (ERROR_INVALID_WINDOW_HANDLE too). I think that
> using robust and straightforward IsWindow() function is better than verify
> last error.
>
The error value was changed in commit
19e7fab981a75cdf0a339db35d36d9c1a6e64494. The code should be changed to
check for ERROR_INVALID_WINDOW_HANDLE now.
> Is it acceptable to check return value of PostMessage only and do not check
> last error?
>
Possibly, although I can think of other errors such as out-of-memory
that probably shouldn't indicate that the parent has disappeared and the
icon destroyed.
> And what race condition introduces IsWindow() here?
>
In general, you shouldn't check whether something can be done before you
try doing it. Just try doing it and cope with it failing.
In this case, you're checking if the window is valid before you post a
message to it. The window could be destroyed between IsWindow and
PostMessage, in which case you'd have to detect the error by checking
the result of the PostMessage call. After you've done this, the IsWindow
call is redundant.
--
Rob Shearman
More information about the wine-devel
mailing list