Re: Test 52 failure on AIX, HP-UX, Solaris

[Joel E. Denny]

On Tue, 2 Feb 2010, Joel E. Denny wrote:

> On Tue, 2 Feb 2010, Albert Chin wrote:
> 
> > On Tue, Feb 02, 2010 at 01:19:38AM -0500, Joel E. Denny wrote:
> > > While the following patch is obviously not a good solution, would you
> > > please confirm that it does allow the test group to pass on the
> > > affected platforms?
> > 
> > Confirmed on RHEL4/x86-64.
> 
> Thanks.
> 
> Here's an attempt at fixing this.  Not yet pushed.  I now need to decide 
> whether this fix is too much for 2.4.2.  If so, I suppose I could just 
> comment out the affected test group for 2.4.2,

I'm rewriting the patch, and I'll figure out what goes in what release 
later....

> but I'm suspicious that 
> there's a race condition that could sometimes affect other test groups.  

Never mind.  I realize now that all other test groups that produce fatal 
errors during m4 use b4_fatal or b4_fatal_at, which make sure that m4 
actually terminates and writes no additional output after printing the 
command that tells scan_skel to invoke fatal or fatal_at.  That is, none 
of our other existing test groups should be affected, but our test suite 
ought to be extended to check other cases where the problem Albert 
reported can happen.

Actually, I can reproduce the problem now, so I've confirmed that only 
test group 52 is affected.  I did this by altering m4 1.4.13's main 
routine to set:

  signal (SIGPIPE, SIG_IGN);

Thus, instead of terminating with a SIGPIPE, m4 now sees fwrite fail with 
errno=EPIPE, and then m4 reports the error that Albert saw.  So the 
platform difference is the default disposition of SIGPIPE.

> * src/output.c (output_skeleton): On a fatal error from
> scan_skel, reap m4 process before exiting.
> * src/scan-skel.h (scan_skel): Update prototype.
> * src/scan-skel.l (skel_lex): Delay fatal errors and return
> false iff there were any.
> (scan_skel): Propagate skel_lex's return.
> (at_directive_perform): Delay fatal errors and return false iff
> there were any.  Update output_file_name_check invocation.
> (fail_for_at_directive_too_few_args): Delay fatal error.
> (fail_for_at_directive_too_many_args): Delay fatal error.
> (fail_for_invalid_at): Delay fatal error.

I dislike this do-it-yourself-try-catch approach.  In general, using 
fatal_delayed and fatal_at_delayed seems error-prone.  I'd rather just 
have fatal and fatal_at handle the pipe directly.

> -  scan_skel (in);
> -  xfclose (in);
> -  reap_subpipe (pid, m4);
> +  {
> +    bool fatal_error = !scan_skel (in);
> +    xfclose (in);
> +    reap_subpipe (pid, m4, fatal_error);
> +    if (fatal_error)
> +      exit (EXIT_FAILURE);
> +  }

Well, that's broken.  I still closed Bison's end of m4's output pipe 
before draining it, so SIGPIPE can still happen.  As I recall, reaping the 
process is actually not important at all when the parent process is about 
to exit anyway.