MySQL Bugs: #47731: mtr freezes for many seconds when process to be killed has already gone.

Bug #47731	mtr freezes for many seconds when process to be killed has already gone.
Submitted:	29 Sep 2009 22:14	Modified:	18 Dec 2009 23:45
Reporter:	Vladislav Vaintroub	Email Updates:
Status:	Closed	Impact on me:	None
Category:	Tools: MTR / mysql-test-run	Severity:	S3 (Non-critical)
Version:		OS:	Windows
Assigned to:	Vladislav Vaintroub	CPU Architecture:	Any

Description:
Quite often , e.g when running rpl test and when quick , often a skipped tests needs to run, mtr freezes for ~10 seconds, then spits out 

"Could not open event or process #"
then after 10 seconds
"Failed to open shutdown_event".

The process to be killed is gone at that time , so there is no wonder that shutdown event cannot be opened

How to repeat:
running rpl test suite almost certainly brings those messages.

Suggested fix:
safe_kill_process_win fails to recognize the process is already dead.
if OpenProcess() succeeds, tit does not automatically mean process is alive.
The OpenProcess() will first fail, when the last handle to this process is closed, not directly after the process dies .

So there still needs to be a check if process is alive with 
GetExitCodeProcess () == STILL_ALIVE

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/85102

2837 Vladislav Vaintroub	2009-09-30
      Bug #47731 mtr freezes for many seconds when process to be killed has already gone. 
      
      The problem is that safe_kill_win fails to detect a dead process. OpenProcess() will 
      succeed even after the process died, it will first fail after the last handle to process 
      is closed.
      
      To fix the problem, check process status with GetExitCodeProcess() and consider 
      process to be dead if the exit code returned by this routine is not STILL_ALIVE.

Looks good. Should I apply patch and test?

I presume you have tested this, looks reasonable to me (not being familiar with the particulars of those Windows system calls)

Tested it last night, it works as advertised.

Only thing I noted in the code was why exit(1) is used if GetExitCodeProcess fails? That would mean that we assume the process is gone and that's why we can't get the return code. Well, seems fair enough.

Pushed into 6.0.14-alpha (revid:alik@sun.com-20091022201524-0efl2fzebfuuf0vk) (version source revid:bjorn.munch@sun.com-20091008122436-bqmy3lmr9iaw4d4s) (merge vers: 6.0.14-alpha) (pib:13)

Pushed into 5.5.0-beta (revid:alik@sun.com-20091022201318-jfvtrzd6lb07cwp5) (version source revid:bjorn.munch@sun.com-20091008093442-or0trmzmb2pqcyk7) (merge vers: 5.4.5-beta) (pib:13)

Test suite change. No changelog entry needed.

Setting report to NDI pending push into 5.1.x.

Pushed into 5.1.41 (revid:bjorn.munch@sun.com-20091021073307-ummbh6668hvfxqjv) (version source revid:bjorn.munch@sun.com-20091021073307-ummbh6668hvfxqjv) (merge vers: 5.1.41) (pib:13)

Test suite change. No changelog entry needed.

Pushed into 5.1.41-ndb-7.1.0 (revid:jonas@mysql.com-20091218102229-64tk47xonu3dv6r6) (version source revid:jonas@mysql.com-20091218095730-26gwjidfsdw45dto) (merge vers: 5.1.41-ndb-7.1.0) (pib:15)

Pushed into 5.1.41-ndb-6.2.19 (revid:jonas@mysql.com-20091218100224-vtzr0fahhsuhjsmt) (version source revid:jonas@mysql.com-20091217101452-qwzyaig50w74xmye) (merge vers: 5.1.41-ndb-6.2.19) (pib:15)

Pushed into 5.1.41-ndb-6.3.31 (revid:jonas@mysql.com-20091218100616-75d9tek96o6ob6k0) (version source revid:jonas@mysql.com-20091217154335-290no45qdins5bwo) (merge vers: 5.1.41-ndb-6.3.31) (pib:15)

Pushed into 5.1.41-ndb-7.0.11 (revid:jonas@mysql.com-20091218101303-ga32mrnr15jsa606) (version source revid:jonas@mysql.com-20091218064304-ezreonykd9f4kelk) (merge vers: 5.1.41-ndb-7.0.11) (pib:15)