Re: [Boost-build] Patch for bjam's process termination waiting on Windows.

From: Vladimir Prus (ghost_at_[hidden])
Date: 2008-04-07 03:44:48

On Sunday 20 January 2008 23:38:36 Jurko Gospodnetić wrote:

First a question -- what is the 259 value? You mentioned MSDN article,
do you have a link?

> Index: execnt.c
> ===================================================================
> --- execnt.c    (revision 42883)
> +++ execnt.c    (working copy)
> @@ -479,12 +479,12 @@
> Â  Â  Â /* wait for a command to complete, while snarfing up any output */
> Â  Â  Â do
> Â  Â  Â {
> + Â  Â  Â  Â /* check for a complete command, briefly */
> + Â  Â  Â  Â i = try_wait(500);
> Â  Â  Â  Â  Â /* read in the output of all running commands */
> Â  Â  Â  Â  Â read_output();
> Â  Â  Â  Â  Â /* close out pending debug style dialogs */
> Â  Â  Â  Â  Â close_alerts();
> - Â  Â  Â  Â /* check for a complete command, briefly */
> - Â  Â  Â  Â if ( i < 0 ) i = try_wait(500);
> Â  Â  Â  Â  Â /* check if a command ran out of time */
> Â  Â  Â  Â  Â if ( i < 0 ) i = try_kill_one();
> Â  Â  Â }
> @@ -499,7 +499,7 @@
> Â  Â  Â  Â  Â /* the time data for the command */
> Â  Â  Â  Â  Â record_times(cmdtab[i].pi.hProcess, &time);
> Â 
> - Â  Â  Â  Â /* Clear the temp file */
> + Â  Â  Â  Â /* clear the temp file */
> Â  Â  Â  Â  Â if ( cmdtab[i].tempfile_bat )
> Â  Â  Â  Â  Â {
> Â  Â  Â  Â  Â  Â  Â unlink( cmdtab[ i ].tempfile_bat );
> @@ -507,6 +507,9 @@
> Â  Â  Â  Â  Â  Â  Â cmdtab[i].tempfile_bat = NULL;
> Â  Â  Â  Â  Â }
> Â 
> + Â  Â  Â  Â /* find out the process exit code */
> + Â  Â  Â  Â GetExitCodeProcess( cmdtab[i].pi.hProcess, &cmdtab[i].exitcode );
> +
> Â  Â  Â  Â  Â /* the dispossition of the command */
> Â  Â  Â  Â  Â if( intr )
> Â  Â  Â  Â  Â  Â  Â rstat = EXEC_CMD_INTR;
> @@ -884,43 +887,41 @@
> Â  Â  Â }
> Â }
> Â 
> -/* Â waits for a single child process command to complete, or the
> - Â  Â timeout, whichever is first. returns the index of the completed
> - Â  Â command, or -1. */
> +/* Â waits for a single child process command to complete, or the timeout,
> + Â  Â whichever comes first. returns the index of the completed command, or -1. */

If we're cleaning the comments, how about this:

/*  Waits for a single child process command to complete, or the timeout,
    whichever comes first. Returns the index of the completed command in the
    cmdtab array, or -1. */

> Â static int try_wait(int timeoutMillis)
> Â {
> - Â  Â int i, num_active, waiting;
> + Â  Â int i;
> + Â  Â int num_active;
> + Â  Â int wait_api_result;
> Â  Â  Â HANDLE active_handles[MAXJOBS];
> Â  Â  Â int active_procs[MAXJOBS];
> Â 
> - Â  Â for ( waiting = 1; waiting; Â )
> + Â  Â /* prepare a list of all active processes to wait for */
> + Â  Â for ( num_active = 0, i = 0; i < globs.jobs; ++i )
> Â  Â  Â {
> - Â  Â  Â  Â /* find the first completed child process */
> - Â  Â  Â  Â for ( num_active = 0, i = 0; i < globs.jobs; ++i )
> + Â  Â  Â  Â if ( cmdtab[i].pi.hProcess )
> Â  Â  Â  Â  Â {
> - Â  Â  Â  Â  Â  Â /* if we have an already dead process, return it. */
> - Â  Â  Â  Â  Â  Â cmdtab[i].exitcode = 0;
> - Â  Â  Â  Â  Â  Â if ( GetExitCodeProcess( cmdtab[i].pi.hProcess, &cmdtab[i].exitcode ) )
> - Â  Â  Â  Â  Â  Â {
> - Â  Â  Â  Â  Â  Â  Â  Â if ( STILL_ACTIVE != cmdtab[i].exitcode )
> - Â  Â  Â  Â  Â  Â  Â  Â {
> - Â  Â  Â  Â  Â  Â  Â  Â  Â  Â return i;
> - Â  Â  Â  Â  Â  Â  Â  Â }
> - Â  Â  Â  Â  Â  Â }
> - Â  Â  Â  Â  Â  Â /* it's running, add it to the list to watch for */
> Â  Â  Â  Â  Â  Â  Â active_handles[num_active] = cmdtab[i].pi.hProcess;
> Â  Â  Â  Â  Â  Â  Â active_procs[num_active] = i;
> - Â  Â  Â  Â  Â  Â num_active += 1;
> + Â  Â  Â  Â  Â  Â ++num_active;
> Â  Â  Â  Â  Â }
> - Â  Â  Â  Â 
> - Â  Â  Â  Â /* wait for a child to complete, or for our timeout window to expire */
> - Â  Â  Â  Â if ( waiting )
> - Â  Â  Â  Â {
> - Â  Â  Â  Â  Â  Â WaitForMultipleObjects( num_active, active_handles, FALSE, timeoutMillis );
> - Â  Â  Â  Â  Â  Â waiting = 0;
> - Â  Â  Â  Â }
> Â  Â  Â }
> - Â  Â 
> +
> + Â  Â /* wait for a child to complete, or for our timeout window to expire */
> + Â  Â wait_api_result = WaitForMultipleObjects( num_active, active_handles,
> + Â  Â  Â  Â FALSE, timeoutMillis );
> + Â  Â if
> + Â  Â (
> + Â  Â  Â  Â ( WAIT_OBJECT_0 <= wait_api_result ) &&
> + Â  Â  Â  Â ( wait_api_result < WAIT_OBJECT_0 + num_active )
> + Â  Â )

Funny indentation you have here :-) Can opening paren go to the same
line as "if" and closing one don't go on a separate line?

> + Â  Â {
> + Â  Â  Â  Â /* terminated process detected - return its index */
> + Â  Â  Â  Â return active_procs[ wait_api_result - WAIT_OBJECT_0 ];
> + Â  Â }
> +
> + Â  Â /* timeout */
> Â  Â  Â return -1;
> Â }
> Â 
> @@ -941,9 +942,7 @@
> Â  Â  Â  Â  Â  Â  Â  Â  Â close_alert(cmdtab[i].pi.hProcess);
> Â  Â  Â  Â  Â  Â  Â  Â  Â /* we have a "runaway" job, kill it */
> Â  Â  Â  Â  Â  Â  Â  Â  Â kill_process_tree(0,cmdtab[i].pi.hProcess);
> - Â  Â  Â  Â  Â  Â  Â  Â /* and return it as complete, with the failure code */
> - Â  Â  Â  Â  Â  Â  Â  Â GetExitCodeProcess( cmdtab[i].pi.hProcess, &cmdtab[i].exitcode );
> - Â  Â  Â  Â  Â  Â  Â  Â /* mark it as a timeout */
> + Â  Â  Â  Â  Â  Â  Â  Â /* and return it marked as a timeout */
> Â  Â  Â  Â  Â  Â  Â  Â  Â cmdtab[i].exit_reason = EXIT_TIMEOUT;
> Â  Â  Â  Â  Â  Â  Â  Â  Â return i;
> Â  Â  Â  Â  Â  Â  Â }

From my limited knowledge of windows API, this seems to be OK. Can you please
commit the patch?

Thanks,
Volodya