bugmake - Bugs: bug #60774, make hangs on fcntl lock when...

 
 

bug #60774: make hangs on fcntl lock when using -O and stdout is /dev/null

Submitter:  Mike Frysinger <vapier>
Submitted:  Fri 11 Jun 2021 10:24:02 PM UTC
   
 
Severity:  3 - Normal Item Group:  None
Status:  Fixed Privacy:  Public
Assigned to:  None Open/Closed:  Closed
Component Version:  4.0 Operating System:  None
Fixed Release:  4.4 Triage Status:  Medium Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 31 Aug 2022 01:59:56 AM UTC, comment #10: 

Thanks for the patch Dmitry.  I decided to do a major overhaul of this area of the code and create os-specific implementations rather than continue to try to emulate POSIX functions on Windows with fcntl.

On Windows the implementation uses a mutex, as before.

On POSIX systems we create a temporary file and then pass the filename to child makes and all the makes lock that file instead of stdout.

Paul D. Smith <psmith>
Group administrator
Sun 25 Jul 2021 04:43:10 PM UTC, comment #9: 

i was able to test on windows and attached a proposed patch.
The patch does what is described in update 3.

Dmitry Goncharov <dgoncharov>
Sun 25 Jul 2021 07:49:03 AM UTC, comment #8: 

Why did you remove the code which reused the mutex passed from the parent Make?

Eli Zaretskii <eliz>
Group Member
Sat 24 Jul 2021 08:56:41 PM UTC, comment #7: 

The windows code, which passes down the semaphore to children is reused to pass down the fd on unix.

> The way you propose to reuse the Windows code brings that Windows-specific stuff back into view.


it is not clear to me what windows specific stuff you mean.
Let us go over it?

record_sync_mutex reads the value of the fd from the env variable. Reused between unix and windows.

prepare_mutex_handle_string stores the value of the fd in makeflags. Reused between unix and windows.

create_mutex is modified to return the value of the semaphore. Still windows specific.

mutex_handle - global variable. Removed. Instead of mutex_handle and sync_handle, there is only sync_handle.


Dmitry Goncharov <dgoncharov>
Sat 24 Jul 2021 06:24:26 PM UTC, comment #6: 

The idea of keeping the Windows code separate was to avoid cluttering common sources with Windows-specific stuff.  The way you propose to reuse the Windows code brings that Windows-specific stuff back into view.

But if Paul doesn't mind, neither do I.

Eli Zaretskii <eliz>
Group Member
Sat 24 Jul 2021 03:25:47 PM UTC, comment #5: 

The MS-Windows port of GNU Make doesn't lock stdout (or any other standard device).  Instead, it uses a mutex to synchronize output.  So I think this problem cannot happen on Windows.

But I see that your changeset touches quite a few places in the code which is Windows specific, and I wonder why did you have to do that, since the problem you are trying to fix doesn't exist there.  Wouldn't it be more prudent to leave the Windows-only code alone?

Eli Zaretskii <eliz>
Group Member
Sat 24 Jul 2021 02:57:17 PM UTC, comment #4: 

In your case, Mike, you can either try this fix or find the offending process with lsof and kill it.

Dmitry Goncharov <dgoncharov>
Sat 24 Jul 2021 02:54:50 PM UTC, comment #3: 

i pushed a fix here
https://github.com/dgoncharov/make/tree/sv_60774_output_sync_deadlock.
This fix does the following.

Lock a temp file to synchronize output.

1. Lock a temp file, rather than stdout, to synchronize output.
Locking stdout prevents make from obtaining the lock when another process has make's stdout locked. E.g make's stdout is redirected to /dev/null and another process has /dev/null locked.

2. Pass the file descriptor of this lock file to children through env in MAKEFLAGS' sync-mutex.

3. Check that a file can be locked before locking it.  With a temp file this check in acquire_semaphore is not supposed to fail. It won't hurt though.

4. Print a warning message when cannot synchronize output.

Tested this fix on linux. Will be good if anybody could test this fix on windows.

Dmitry Goncharov <dgoncharov>
Sat 24 Jul 2021 02:51:29 PM UTC, comment #2: 

Thanks for your report, Mike.

make uses file locking on stdout to synchronize output.

int fd = fileno(STDOUT);
fcntl (fd, F_SETLKW, &fl);

After fcntl return make prints its output.

In your case another process has /dev/null locked. With make's stdout redirected to /dev/null, make blocks in this call to fcntl.

Dmitry Goncharov <dgoncharov>
Wed 21 Jul 2021 03:55:20 PM UTC, comment #1: 

i thought i had included it originally, but i think it might be a change in the kernel version.  i'm testing against linux-5.10 & glibc-2.31.

Mike Frysinger <vapier>
Fri 11 Jun 2021 10:24:02 PM UTC, original submission:  

i can reproduce this with make 4.2.1, 4.3, and current git (012918bf11fbc2a94dc2319d15d05595c351b811) on Debian/Ubuntu systems.  my reproduction:

$ wget https://busybox.net/downloads/busybox-1.32.1.tar.bz2
$ tar xf busybox-1.32.1.tar.bz2
$ cd busybox-1.32.1
# This works fine.
$ make -O -j32 defconfig
# This hangs indefinitely.
$ make -O -j32 defconfig >/dev/null

gdb shows the hang in:

(gdb) bt
#0  0x00007f10555c9643 in fcntl64 () from /lib64/libc.so.6
#1  0x00000000002225ff in acquire_semaphore () at output.c:267
#2  output_dump (out=out@entry=0x13a9744) at output.c:368
#3  0x000000000021b43a in reap_children (block=block@entry=0x1, err=<optimized out>, err@entry=0x0) at job.c:937
#4  0x0000000000227c99 in update_goal_chain (goaldeps=<optimized out>) at remake.c:112
#5  0x000000000021fa1b in main (argc=0x5, argv=0x7ffdd42b3478, envp=0x7ffdd42b34a8) at main.c:2558

(gdb) f 1
#1  0x00000000002225ff in acquire_semaphore () at output.c:267
267       if (fcntl (sync_handle, F_SETLKW, &fl) != -1)

(gdb) list
262
263       fl.l_type = F_WRLCK;
264       fl.l_whence = SEEK_SET;
265       fl.l_start = 0;
266       fl.l_len = 1;
267       if (fcntl (sync_handle, F_SETLKW, &fl) != -1)
268         return &fl;
269       perror ("fcntl()");
270       return NULL;
271     }

strace confirms it:
286807 fcntl(1, F_SETLKW, {l_type=F_WRLCK, l_whence=SEEK_SET, l_start=0, l_len=1}

full strace log attached in case it helps.

Mike Frysinger <vapier>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #51549:  log.xz added by vapier (443KiB - application/octet-stream)

 

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 eliz (Posted a comment)
  • -email is unavailable- added by dgoncharov (Posted a comment)
  • -email is unavailable- added by vapier (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-08-31 psmith StatusNone Fixed
        Open/ClosedOpen Closed
        Component VersionNone 4.0
        Fixed ReleaseNone 4.4
        Triage StatusNone Medium Effort
    2021-07-25 dgoncharov Attached File- Added sv_60774_output_sync_deadlock_fix2.diff, #51689
    2021-07-25 dgoncharov Attached File- Added sv_60774_output_sync_deadlock_fix.diff, #51688
    2021-06-11 vapier Attached File- Added log.xz, #51549

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code