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:
- run any wine app (to be sure that explorer is running), e.g winecfg
- run taskmgr
- 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.