bugmake - Bugs: bug #46261, missing usage of EINTRLOOP macro

 
 

bug #46261: missing usage of EINTRLOOP macro

Submitter:  Jack Howarth <jwhowarth>
Submitted:  Wed 21 Oct 2015 02:02:48 AM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  4.1 Operating System:  POSIX-Based
Fixed Release:  4.2 Triage Status:  Medium Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 09 Mar 2016 04:46:00 AM UTC, comment #12: 

I modified the implementation of the jobserver function to use pselect() on systems where it's supported.  This avoids the EINTR problem as it allows us to block SIGCHLD throughout the process except for (atomically) within the pselect() system call itself, so we know we won't lose signals.

Paul D. Smith <psmith>
Group administrator
Sat 24 Oct 2015 03:02:34 PM UTC, comment #11: 

I finally pinned down the actual cause of these regression in parallel builds on 10.11 with make 4.1 The issue was being triggered by the default usage of --enable-nls in the build. This drags in a libintl which is built against the CoreFoundation framework on darwin. The regressions on 10.11 can be either suppressed by building make 4.1 with the --disable-nls option or rebuilding gettext using the....

        gt_cv_func_CFPreferencesCopyAppValue=no \
        gt_cv_func_CFLocaleCopyCurrent=no \
        gt_cv_func_CFPreferencesCopyAppValue=no \
        gt_cv_func_CFLocaleCopyCurrent=no

in order to prevent linkage of libintl against the CoreFoundation framework. This also corresponds with the known behavior of pymol which uses fork()/exec() under tcl. On darwin, tcl has to be built with --disable-corefoundation to avoid crashes in pymol. Looking at the CoreFoundation framework sources, there doesn't appear to be any effort to handle EINTR error codes on read(), write() and (f)stat() calls.

Jack Howarth <jwhowarth>
Fri 23 Oct 2015 08:22:51 PM UTC, comment #10: 

I notice that gettext-0.19.5.1/gettext-runtime/intl/loadmsgcat.c has...

 __builtin_expect (fstat (fd, &st) != 0, 0)

which doesn't seem to provide any EINTR checking.

Jack Howarth <jwhowarth>
Fri 23 Oct 2015 07:31:53 PM UTC, comment #9: 

I finally puzzled out that this regression in the builds of gcc 5.2.0 and openmpi can be suppressed by rebuilding make 4.1 with --disable-nls. This actually makes sense as there seems to be be incomplete usage of the nonintr_read and nonintr_write calls that check for EINTR in gettext-0.19.5.1/gettext-tools/gnulib-lib. Also I don't see any matching nonintr_stat/nonintr_fstat calls in gettext-0.19.5.1/gettext-tools/gnulib-lib or checks for EINTR when stat()/fstat() is used there.

Jack Howarth <jwhowarth>
Thu 22 Oct 2015 05:39:00 AM UTC, comment #8: 

The remaining failure in the build of FSF gcc 5.2.0 with make 4.1 on OS X 10.11 under perl may actually be a latent bug in the libjava Makefiles being re-exposed. It is identical to one that I previously reported...

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63827#c5

but which went latent in later gcc trunk.

Jack Howarth <jwhowarth>
Wed 21 Oct 2015 03:57:54 PM UTC, comment #7: 

using

EINTRLOOP(job_rfd, dup (job_fds[0]));

instead of

job_rfd = dup (job_fds[0]);

in the new_job() subroutine of job.c results in a  hang during the parallel build of gcc 5.2.0 rather than the previous error...

make[5]: * read jobs pipe: No such file or directory. Stop.

Jack Howarth <jwhowarth>
Wed 21 Oct 2015 03:05:32 PM UTC, comment #6: 

How do we know that the earlier call to duplicate the file descriptor in the new_job() subroutine of job.c succeeds?

        /* Make sure we have a dup'd FD.  */
        if (job_rfd < 0)
          {
            DB (DB_JOBS, ("Duplicate the job FD\n"));
            job_rfd = dup (job_fds[0]);
          }

There does't be a check if -1 was returned to job_rd or any error code placed on the global errno.

Jack Howarth <jwhowarth>
Wed 21 Oct 2015 02:45:05 PM UTC, comment #5: 

The full error that we are seeing on OS X 10.11 under make 4.1 called from perl after a flock() is...

make[5]: * read jobs pipe: No such file or directory.  Stop.
make[5]: * Waiting for unfinished jobs....
libtool: compile:  /sw/src/fink.build/gcc5-5.2.0-2/darwin_objdir/./gcc/gcj -B/sw/src/fink.build/gcc5-5.2.0-2/darwin_objdir/x86_64-apple-darwin15.0.0/i386/libjava/ -B/sw/src/fink.build/gcc5-5.2.0-2/darwin_objdir/x86_64-apple-darwin15.0.0/i386/libjava/ -B/sw/src/fink.build/gcc5-5.2.0-2/darwin_objdir/./gcc/ -B/sw/lib/gcc5/x86_64-apple-darwin15.0.0/bin/ -B/sw/lib/gcc5/x86_64-apple-darwin15.0.0/lib/ -isystem /sw/lib/gcc5/x86_64-apple-darwin15.0.0/include -isystem /sw/lib/gcc5/x86_64-apple-darwin15.0.0/sys-include -m32 -ffloat-store -fomit-frame-pointer -Usun -fclasspath= -fbootclasspath=../../../../gcc-5.2.0/libjava/classpath/lib --encoding=UTF-8 -Wno-deprecated -fbootstrap-classes -g -O2 -m32 -c --resource gnu/java/locale/LocaleInformation_gaa.properties ../../../../gcc-5.2.0/libjava/classpath/resource/gnu/java/locale/LocaleInformation_gaa.properties  -fno-common -o gnu/java/locale/.libs/LocaleInformation_gaa.properties.o
Makefile:10240: recipe for target 'all-recursive' failed
make[4]: * [all-recursive] Error 1
make[4]: Leaving directory '/sw/src/fink.build/gcc5-5.2.0-2/darwin_objdir/x86_64-apple-darwin15.0.0/i386/libjava'
Makefile:12804: recipe for target 'multi-do' failed
make[3]: * [multi-do] Error 1
make[3]: Leaving directory '/sw/src/fink.build/gcc5-5.2.0-2/darwin_objdir/x86_64-apple-darwin15.0.0/libjava'
Makefile:12768: recipe for target 'all-multi' failed
make[2]: * [all-multi] Error 2
make[2]: Leaving directory '/sw/src/fink.build/gcc5-5.2.0-2/darwin_objdir/x86_64-apple-darwin15.0.0/libjava'
Makefile:17475: recipe for target 'all-target-libjava' failed
make[1]: * [all-target-libjava] Error 2
Makefile:21086: recipe for target 'bootstrap-lean' failed
make: * [bootstrap-lean] Error 141
make: INTERNAL: Exiting with 1 jobserver tokens available; should be 8!

in the parallel make  FSF gcc 5.2.0 build. My understanding is that the error code 141 is an error above 128 indicating a termination due to signal which in this case would be 141-128=13 or "Broken pipe: write to pipe with no readers".

Jack Howarth <jwhowarth>
Wed 21 Oct 2015 02:01:47 PM UTC, comment #4: 

The offending code...

#else
        /* Set interruptible system calls, and read() for a job token.  */
        set_child_handler_action_flags (1, waiting_jobs != NULL);
        got_token = read (job_rfd, &token, 1);
        saved_errno = errno;
        set_child_handler_action_flags (0, waiting_jobs != NULL);

in new_job() of job.c doesn't seem suitable for applying the EINTRLOOP() macro. Changing the line for....

 got_token = read (job_rfd, &token, 1);

to

EINTRLOOP(got_token, read (job_rfd, &token, 1));

converts the gcc 5.2.0 build failure from...

make[5]: * read jobs pipe: No such file or directory. Stop.

to an Error 141 code.

Jack Howarth <jwhowarth>
Wed 21 Oct 2015 01:11:31 PM UTC, comment #3: 

There seems to still be missing EINTRLOOP () macro usage on OS X 10.11. The gcc 5.2.0 parallel build still fails but the initial error in its case is...

make[5]: * read jobs pipe: No such file or directory.  Stop.

...ending with...

make: INTERNAL: Exiting with 1 jobserver tokens available; should be 8!

so I am guessing we need a EINTRLOOP  wrapper on...

got_token = read (job_rfd, &token, 1);

in new_job() of job.c as well.

Jack Howarth <jwhowarth>
Wed 21 Oct 2015 02:48:35 AM UTC, comment #2: 

Of course that should be 10.11 and not 10.12 for the OS X version.

Jack Howarth <jwhowarth>
Wed 21 Oct 2015 02:18:40 AM UTC, comment #1: 

The change...

diff -uNr make-4.1.orig/main.c make-4.1/main.c
--- make-4.1.orig/main.c        2014-10-05 12:24:51.000000000 -0400
+++ make-4.1/main.c     2015-10-20 22:08:00.000000000 -0400
@@ -3364,9 +3364,12 @@
 #else
       /* Close the write side, so the read() won't hang.  */
       close (job_fds[1]);
-
-      while (read (job_fds[0], &token, 1) == 1)
+      int r;
+      EINTRLOOP (r, read (job_fds[0], &token, 1));
+      while (r == 1) {
         ++tcnt;
+       EINTRLOOP (r, read (job_fds[0], &token, 1));
+      }
 #endif
 
       if (tcnt != master_job_slots)

seems to eliminate the failures here on OS X 10.12.

Jack Howarth <jwhowarth>
Wed 21 Oct 2015 02:02:48 AM UTC, original submission:  

The fink project has found that their package manager (which uses flock() under perl) triggers a race condition in make 4.1 producing build failures of the form...

make: INTERNAL: Exiting with 1 jobserver tokens available; should be 8!

on OS X 10.12.

The following code in main.c is suspected...

3365       /* Close the write side, so the read() won't hang.  */
3366       close (job_fds[1]);
3367
3368       while (read (job_fds[0], &token, 1) == 1)
3369         ++tcnt;
3370 #endif
3371
3372       if (tcnt != master_job_slots)
3373         ONN (error, NILF,
3374              "INTERNAL: Exiting with %u jobserver tokens available; should be %u!",
3375              tcnt, master_job_slots);
3376
3377 #ifdef WINDOWS32
3378       free_jobserver_semaphore ();
3379 #else
3380       close (job_fds[0]);
3381 #endif
3382

as the section...

while (read (job_fds[0], &token, 1) == 1)
    ++tcnt;

seems to be missing the usage of the EINTRLOOP() macro and the reads could fail with EINTR resulting in tcnt being left at 1 as observed when make 4.1 is executed from perl with prior flock() usage in the perl scripts.
     Any idea how to adjust those two lines to properly use the  EINTRLOOP() macro?

Jack Howarth <jwhowarth>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #35248:  make.patch added by jwhowarth (544B - application/octet-stream - proposed fix)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by jwhowarth (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-03-09 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.2
        Triage StatusNone Medium Effort
    2015-10-21 jwhowarth Attached File- Added make.patch, #35248

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code