[CMake] [PATCH] KWSys: Avoid buggy SIGCHLD on Interix

[Markus Duft]

Brad King wrote:
> On Interix we cannot rely on SIGCHLD because it is only sometimes
> delivered.  Our SIGCHLD handler wakes up any current select() call to
> check if its child terminated, but the handler itself does not care
> about any child.  Therefore we can use the same handler with SIGALRM to
> poll at regular intervals.

the patch is ok as-is now. it fixes my issues on multi-core interix. in
combination with my interix patch (attached again), interix seems to
work rather good. i get the following test results (which i find
surprisingly good, knowing of the many hickups interix can have ;)):

97% tests passed, 5 tests failed out of 156

Total Test time (real) = 2058.00 sec

The following tests FAILED:
         49 - ExportImport (Failed)
         73 - CMakeTestMultipleConfigures (SEGFAULT)
         96 - Plugin (Failed)
        106 - CTest.UpdateGIT (Failed)
        123 - CTestTestTimeout (Failed)
Errors while running CTest

Opinions?

Thanks!

Cheers, Markus

> 
> Patch from Markus Duft, posted to cmake at cmake.org, 2010-02-18.
> ---
> 
> Markus Duft wrote:
>>> with this check it seems that all sub-processes "time out" immediately.
> 
> On this front, it turns out that you're correct.  The polling timeout
> code path was wrong...in CMake 2.6.4.  When I responded before I was
> looking at the latest development sources which have already fixed the
> problem by calling GetTimeoutLeft correctly.
> 
>>> so now i have two patches to make it work - which one should i prefer?
> 
> Here is a patch based on your sigalrm patch.  We can get better than
> 1 second intervals by using "uslarm()", which I hope exists on Interix.
> 
> Please build CMake 2.8.0 with this patch and try it.
> 
> -Brad
> 
>  Source/kwsys/ProcessUNIX.c |   29 +++++++++++++++++++++++++++++
>  1 files changed, 29 insertions(+), 0 deletions(-)
> 
> diff --git a/Source/kwsys/ProcessUNIX.c b/Source/kwsys/ProcessUNIX.c
> index 373e906..a70c172 100644
> --- a/Source/kwsys/ProcessUNIX.c
> +++ b/Source/kwsys/ProcessUNIX.c
> @@ -2526,6 +2526,13 @@ static kwsysProcessInstances kwsysProcesses;
>  /* The old SIGCHLD handler.  */
>  static struct sigaction kwsysProcessesOldSigChldAction;
> 
> +#if defined(__INTERIX)
> +/* Interix does not deliver SIGCHLD reliably when a child terminates
> +   so we simulate it at regular intervals using SIGALRM.  */
> +# define KWSYSPE_SIGALRM_INTERVAL 100000 /* usec */
> +static struct sigaction kwsysProcessesOldSigAlrmAction;
> +#endif
> +
>  /*--------------------------------------------------------------------------*/
>  static void kwsysProcessesUpdate(kwsysProcessInstances* newProcesses)
>  {
> @@ -2536,6 +2543,9 @@ static void kwsysProcessesUpdate(kwsysProcessInstances* newProcesses)
>    sigset_t oldset;
>    sigemptyset(&newset);
>    sigaddset(&newset, SIGCHLD);
> +#if defined(KWSYSPE_SIGALRM_INTERVAL)
> +  sigaddset(&newset, SIGALRM);
> +#endif
>    sigprocmask(SIG_BLOCK, &newset, &oldset);
> 
>    /* Store the new set in that seen by the signal handler.  */
> @@ -2642,6 +2652,13 @@ static int kwsysProcessesAdd(kwsysProcess* cp)
>      while((sigaction(SIGCHLD, &newSigChldAction,
>                       &kwsysProcessesOldSigChldAction) < 0) &&
>            (errno == EINTR));
> +
> +#if defined(KWSYSPE_SIGALRM_INTERVAL)
> +    while((sigaction(SIGALRM, &newSigChldAction,
> +                     &kwsysProcessesOldSigAlrmAction) < 0) &&
> +          (errno == EINTR));
> +    ualarm(KWSYSPE_SIGALRM_INTERVAL, KWSYSPE_SIGALRM_INTERVAL);
> +#endif
>      }
>    }
> 
> @@ -2680,6 +2697,12 @@ static void kwsysProcessesRemove(kwsysProcess* cp)
>        while((sigaction(SIGCHLD, &kwsysProcessesOldSigChldAction, 0) < 0) &&
>              (errno == EINTR));
> 
> +#if defined(KWSYSPE_SIGALRM_INTERVAL)
> +      ualarm(0, 0);
> +      while((sigaction(SIGALRM, &kwsysProcessesOldSigAlrmAction, 0) < 0) &&
> +            (errno == EINTR));
> +#endif
> +
>        /* Free the table of process pointers since it is now empty.
>           This is safe because the signal handler has been removed.  */
>        newProcesses.Size = 0;
> @@ -2736,6 +2759,12 @@ static void kwsysProcessesSignalHandler(int signum
>    while((sigaction(SIGCHLD, &newSigChldAction,
>                     &kwsysProcessesOldSigChldAction) < 0) &&
>          (errno == EINTR));
> +
> +#if defined(KWSYSPE_SIGALRM_INTERVAL)
> +  while((sigaction(SIGALRM, &newSigChldAction,
> +                   &kwsysProcessesOldSigAlrmAction) < 0) &&
> +        (errno == EINTR));
> +#endif
>    }
>  #endif
>  }

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: cmake-2.6.0-interix.patch
URL: <http://www.cmake.org/pipermail/cmake/attachments/20100223/18a320a6/attachment.txt>