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

[Paul D. Smith]

Update of bug #60774 (project make):

  Status:None => Fixed  
 Open/Closed:Open => Closed 
   Component Version:None => 4.0
   Fixed Release:None => SCM
   Triage Status:None => Medium Effort  

___

Follow-up 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.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

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

[Paul Smith]

On Sun, 2021-07-25 at 17:29 -0700, David Boyce wrote:
> It's simple to detect this on a Unix-like host (see sample script
> below) and my contention is that this is "plenty portable enough"

Ah.  Yes, determining if stdout is /dev/null can be done.  I was
definitely having an off day yesterday and forgot about fstat()!

Thanks for that reminder.

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

[David Boyce]

Paul,

I don't understand why you say "I don't know of any portable way of
determining whether stdout is going to /dev/null, or not" and "However,
it's moot because I don't think we can detect it". It's simple to detect
this on a Unix-like host (see sample script below) and my contention is
that this is "plenty portable enough". The concepts of /dev/null and
device/inode both go right back to the very first days of Unix so I
think it's almost certain that any Unix-derived or -inspired platform is
going to support both. Put differently, I think any system supported by GNU
make will have both or neither. But let's imagine a system that has just
one of the two: for systems that don't have /dev/null, what's the problem?
It can't be locked if it's not there and any system that does implement
/dev/null will certainly implement it with the traditional semantics.
Native Windows doesn't have /dev/null (and Eli confirms that this can't
happen there), Cygwin emulates both, etc. I believe both preconditions are
easily detectable at configure time.

Bottom line, the odds of any platform that supports a lockable /dev/null
not having st_ino in a stat structure, at least an emulated one, seem
vanishingly small to me. Do you know of a platform where the strategy of
"implement the special case for /dev/null if possible or retain current
semantics if not" breaks down?

David

$ cat /tmp/null-vs-stdout
#!/usr/bin/env python3
import os
import sys
for path in sys.argv[1:]:
p = os.stat(path)
s = os.fstat(sys.stdout.fileno())
if p.st_ino == s.st_ino and p.st_dev == s.st_dev:
sys.stderr.write('IS stdout: %s\n' % path)
else:
sys.stderr.write('not stdout: %s\n' % path)

$ python3 /tmp/null-vs-stdout /dev/null
not stdout: /dev/null

$ python3 /tmp/null-vs-stdout /dev/null > /dev/null
IS stdout: /dev/null

On Sun, Jul 25, 2021 at 1:31 PM Paul Smith  wrote:

> On Sun, 2021-07-25 at 16:04 -0400, Dmitry Goncharov wrote:
> > > It's clear that /dev/null is a special case, however, because the
> > > order in which data is written to it cannot be detected so what's
> > > the point in preserving any order?
> >
> > Maybe Mike can tell us.
>
> Based on the bug report I don't think Mike knows.  It didn't appear to
> me that he had purposefully locked /dev/null, or was even aware that it
> was locked.  Is there something else on his system that's doing it?  Is
> it a change to the kernel that cause locks here to hang?  We don't
> know.
>
> > > What I'm saying is that IF make can detect that its stdout is going
> > > to /dev/null then it shouldn't lock at all, because it's not
> > > necessary to do so in order to meet the goals of the -O option, and
> > > doing so introduces unnecessary latency in the build.
> >
> > Even if we come up with a portable mechanism to detect /dev/null,
> > what about other files? What about /dev/stdout or /dev/pts/5? i don't
> > think, it is possible or even reasonable to attempt to special case
> > all these files.
>
> What I'm trying to say is that even if we do special-case /dev/null we
> definitely shouldn't special case other files, because /dev/null is
> truly a unique situation which is fundamentally different from
> /dev/pts/5 etc., and is deserving of special handling.
>
> However, it's moot because I don't think we can detect it.
>
> > > The  question is what to do about supporting -O on these systems.
> > A private temp file will work.
>
> Yes.  It's good to have that capability.
>
> Another option would be to use a POSIX named semaphore.  The advantage
> of this is that you don't have to worry about temporary file creation,
> permissions, and cleaning up.
>
>
>
>

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

[Paul Smith]

On Sun, 2021-07-25 at 16:04 -0400, Dmitry Goncharov wrote:
> > It's clear that /dev/null is a special case, however, because the
> > order in which data is written to it cannot be detected so what's
> > the point in preserving any order?
> 
> Maybe Mike can tell us.

Based on the bug report I don't think Mike knows.  It didn't appear to
me that he had purposefully locked /dev/null, or was even aware that it
was locked.  Is there something else on his system that's doing it?  Is
it a change to the kernel that cause locks here to hang?  We don't
know.

> > What I'm saying is that IF make can detect that its stdout is going
> > to /dev/null then it shouldn't lock at all, because it's not
> > necessary to do so in order to meet the goals of the -O option, and
> > doing so introduces unnecessary latency in the build.
> 
> Even if we come up with a portable mechanism to detect /dev/null,
> what about other files? What about /dev/stdout or /dev/pts/5? i don't
> think, it is possible or even reasonable to attempt to special case
> all these files.

What I'm trying to say is that even if we do special-case /dev/null we
definitely shouldn't special case other files, because /dev/null is
truly a unique situation which is fundamentally different from
/dev/pts/5 etc., and is deserving of special handling.

However, it's moot because I don't think we can detect it.

> > The  question is what to do about supporting -O on these systems.
> A private temp file will work.

Yes.  It's good to have that capability.

Another option would be to use a POSIX named semaphore.  The advantage
of this is that you don't have to worry about temporary file creation,
permissions, and cleaning up.

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

[Dmitry Goncharov via Bug reports and discussion for GNU make]

On Sun, Jul 25, 2021 at 3:25 PM Paul Smith  wrote:
> I have no problem with make refusing to write to a device that's locked
> in general.  If users don't want that to happen, they can just not use
> the -O option.

True, as long as the user knows that when -O is specified make is
going to lock stdout.


> It's clear that /dev/null is a special case, however, because the order
> in which data is written to it cannot be detected so what's the point
> in preserving any order?

Maybe Mike can tell us.


> What I'm saying is that IF make can detect that its stdout is going to
> /dev/null then it shouldn't lock at all, because it's not necessary to
> do so in order to meet the goals of the -O option, and doing so
> introduces unnecessary latency in the build.

Even if we come up with a portable mechanism to detect /dev/null, what
about other files?
What about /dev/stdout or /dev/pts/5?
i don't think, it is possible or even reasonable to attempt to special
case all these files.


> The  question is what to do about supporting -O on these systems.

A private temp file will work.


> That's true but there are, unfortunately, some edge cases where Windows
> and POSIX systems behave differently.

True. Is this case really one of those edge cases?

>  The goal is to provide  the best implementation of make for the GNU system

What do you think we should do about this bug report?

regards, Dmitry

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

[Paul Smith]

On Sun, 2021-07-25 at 15:25 -0400, Paul Smith wrote:
> There's no reason that those two disjoint sets of output need to be
> synchronized WITH EACH OTHER to meet the goal of the -O option. Given
> the choice between allowing output to go to these two different
> locations in parallel vs. serializing them with each other, I don't
> think anyone would prefer the latter.

In any event, what I'm really trying to say is this: locking stdout
rather than some other way of lock sharing was done quite deliberately
and intentionally and does provide tangible benefits that can't be
easily duplicated using other locking methods.  It wasn't just a
careless implementation choice.

Maybe those benefits don't outweigh the problems they cause, or maybe
they do.  But either way it needs to be considered.

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

[Paul Smith]

On Sun, 2021-07-25 at 13:11 -0400, Dmitry Goncharov wrote:
> On Sun, Jul 25, 2021 at 10:03 AM Paul Smith  wrote:
> > Writing to things like /dev/lp0 SHOULD be locked, IMO: you don't
> > want multiple concurrent writers to your printers or your terminal!
> 
> The user intentionally redirects make to a printer along with another
> tool. Should make be smarter than the user and refuse?

I suppose you mean in the same way Mike reported, the "other tool"
locks the device (e.g. printer) and now if you use -O then make's lock
prevents make from writing to it (or vice versa).

I have no problem with make refusing to write to a device that's locked
in general.  If users don't want that to happen, they can just not use
the -O option.  Or if they do want to use the -O option and they really
do want to write to the device even if it's locked, it's trivial enough
to hide it from make by introducing a intermediate step, like this:

  $ make | cat > /dev/lp0

It's clear that /dev/null is a special case, however, because the order
in which data is written to it cannot be detected so what's the point
in preserving any order?

> > To me the most compelling reason to change the current locking
> > behavior is not this: I agree with David that simply special-casing
> > /dev/null could be a good solution; if you're redirecting to
> > /dev/null why even HAVE a lock?
> 
> Mike wanted to specify -O and redirect to /dev/null. Again, why
> should the tool be smarter the user?

What I'm saying is that IF make can detect that its stdout is going to
/dev/null then it shouldn't lock at all, because it's not necessary to
do so in order to meet the goals of the -O option, and doing so
introduces unnecessary latency in the build.

The problem is, I don't know of any portable way of determining whether
stdout is going to /dev/null, or not.  The only way I know of to do it
is to check /proc//fd/1 and that's pretty much just available in
Linux (and requires you to have procfs mounted which not all Linux
systems do).

> > The most compelling reason to change the current behavior is that
> > unfortunately BSD-based systems don't support locking file
> > descriptors that aren't real files: so if you pipe make's output to
> > something on a BSD-based system like MacOS or FreeBSD, for example,
> > you get a ton of error messages.
> 
> One mechanism to avoid these error messages would be to stat stdout
> and check S_IFREG.

Certainly I didn't mean to suggest that detecting this situation is
hard (actually the simplest way is to just check the error code when we
try to take the lock and "do something else" if it fails).  The
question is what to do about supporting -O on these systems.

> >  submake: ; $(MAKE) -C $@ >sub.out
> > The way the implementation is today, the outer make and the submake
> > magically have different "output lock domains",
> 
> This is an interesting situation.
> 
> i considered this behavior to be a violation of the contract. The
> make manual says that when -O is specified the output of the parent
> and children is synchronized.

I don't agree; the -O option tells make that the user wants to see the
output of each rule not mixed with the output of other rules.  That's
all it means: any other deeper or more technical meaning is an accident
of the current implementation and can't be relied on.

Here the output to stdout is synchronized, and the output to the
redirected file is synchronized.

There's no reason that those two disjoint sets of output need to be
synchronized WITH EACH OTHER to meet the goal of the -O option.  Given
the choice between allowing output to go to these two different
locations in parallel vs. serializing them with each other, I don't
think anyone would prefer the latter.

> make is a portable tool and this magic does not happen on windows.

That's true but there are, unfortunately, some edge cases where Windows
and POSIX systems behave differently.  As long as the builds succeed in
largely similar ways and the edge cases don't cause incorrect builds,
we live with them.

My goal is not to provide an implementation of make which supports only
the least common denominator of all platforms.  The goal is to provide
the best implementation of make for the GNU system, and provide best-
effort support for other platforms that people care about as well.

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

[Dmitry Goncharov]

Additional Item Attachment, bug #60774 (project make):

File name: sv_60774_output_sync_deadlock_fix2.diff Size:6 KB
   




___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

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

[Dmitry Goncharov via Bug reports and discussion for GNU make]

On Sun, Jul 25, 2021 at 10:03 AM Paul Smith  wrote:
> Writing to
> things like /dev/lp0 SHOULD be locked, IMO: you don't want multiple
> concurrent writers to your printers or your terminal!

The user intentionally redirects make to a printer along with another
tool. Should make be smarter than the user and refuse?
When -O is not specified make will happily run redirected to a printer.


> To me the most compelling reason to change the current locking behavior
> is not this: I agree with David that simply special-casing /dev/null
> could be a good solution; if you're redirecting to /dev/null why even
> HAVE a lock?

Mike wanted to specify -O and redirect to /dev/null. Again, why should
the tool be smarter the user?


> The most compelling reason to change the current behavior is that
> unfortunately BSD-based systems don't support locking file descriptors
> that aren't real files: so if you pipe make's output to something on a
> BSD-based system like MacOS or FreeBSD, for example, you get a ton of
> error messages.

One mechanism to avoid these error messages would be to stat stdout
and check S_IFREG.


> submake: ; $(MAKE) -C $@ >sub.out
>
> The way the implementation is today, the outer make and the submake
> magically have different "output lock domains",

This is an interesting situation.
i considered this behavior to be a violation of the contract. The make
manual says that when -O is specified the output of the parent and
children is synchronized.
make is a portable tool and this magic does not happen on windows.
i think, we should either fix the deadlock or document that make locks
stdout. Otherwise, you see, the users are confused.


regards, Dmitry

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

[Dmitry Goncharov via Bug reports and discussion for GNU make]

On Sun, Jul 25, 2021 at 3:49 AM Eli Zaretskii  wrote:
>
> Follow-up Comment #8, bug #60774 (project make):
>
> Why did you remove the code which reused the mutex passed from the parent
> Make?

Eli, i don't think i removed any code, other than the global variable
mutex_handle. Atleast, not intentionally.
i moved record_sync_mutex from w32/compat/posixfcn.c to output.c.
Do you think you tell me the function name or file and line number?

regards, Dmitry

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

[Dmitry Goncharov]

Follow-up Comment #9, bug #60774 (project make):

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

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

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

[Dmitry Goncharov]

Additional Item Attachment, bug #60774 (project make):

File name: sv_60774_output_sync_deadlock_fix.diff Size:6 KB
   




___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

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

[Paul Smith]

On Sat, 2021-07-24 at 20:35 -0400, Dmitry Goncharov wrote:
> On Sat, Jul 24, 2021 at 3:02 PM David Boyce 
> wrote:
> > I can’t think of a file other than /dev/null which would
> > appropriately be shared with unrelated processes in a “w” (write)
> > condition.
> 
> /dev/zero, /dev/lp0, /dev/lp1, /dev/pts, etc.

/dev/zero should not be written TO, it's only read from.  Writing to
things like /dev/lp0 SHOULD be locked, IMO: you don't want multiple
concurrent writers to your printers or your terminal!

To me the most compelling reason to change the current locking behavior
is not this: I agree with David that simply special-casing /dev/null
could be a good solution; if you're redirecting to /dev/null why even
HAVE a lock?  Assuming you can determine this it would be better to
simply turn it off and avoid the overhead.

The most compelling reason to change the current behavior is that
unfortunately BSD-based systems don't support locking file descriptors
that aren't real files: so if you pipe make's output to something on a
BSD-based system like MacOS or FreeBSD, for example, you get a ton of
error messages.

> i recognize the simplicity of using stdout. However, i also dislike
> adding pieces of code for a set of special files. The user will
> always find a way to screw it.

Using stdout was chosen intentionally because it gives one big
advantage, which is why I've struggled with the idea of replacing it:

It gives a simple and elegant solution to the situation where a
submake's output is redirected to a different file.  Suppose you have
this:

submake: ; $(MAKE) -C $@ >sub.out

The way the implementation is today, the outer make and the submake
magically have different "output lock domains", because their stdout
goes to different files: the submake doesn't have to wait for the lock
of the outer make to write content to its output, and vice versa.  This
is a huge benefit, and I'm loathe to give it up.

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

[Eli Zaretskii]

Follow-up Comment #8, bug #60774 (project make):

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


___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

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

[Eli Zaretskii]

> From: Dmitry Goncharov 
> Date: Sat, 24 Jul 2021 22:03:37 -0400
> Cc: Mike Frysinger , Paul Smith , 
> bo...@kolpackov.net, 
>   bug-make 
> 
> Wanted to test that windows this still works with this change.
> 
> So, i followed instructions in README.W32.
> Installed mingw gcc. Copied config.h.W32.template to config.h.w32.
> Ran build_W32.bat gcc. It fails with
> 
> C:\dgoncharov\make>gcc -mthreads -Wall -std=gnu99 -gdwarf-2 -g3 -O2 
> -DMAKE_MAINT
> AINER_MODE -I.\GccRel/src -I./src -I.\GccRel/lib -I./lib -I./src/w32/include 
> -DW
> INDOWS32 -DHAVE_CONFIG_H  -o .\GccRel/src/ar.o -c src/ar.c
> In file included from src/ar.c:18:
> src/makeint.h:314:13: error: conflicting types for 'mode_t'
>  typedef int mode_t;
>  ^~
> In file included from src/makeint.h:86,
>  from src/ar.c:18:
> c:/Program Files 
> (x86)/mingw-w64/i686-8.1.0-posix-dwarf-rt_v6-rev0/mingw32/i686-
> w64-mingw32/include/sys/types.h:77:17: note: previous declaration of 'mode_t' 
> wa
> s here  typedef _mode_t mode_t;
> 
> Both make and mingw declare mode_t.
> More errors like this follow.
> 
> Does anyone have a ready to use config.h.W32?

config.h.W32.template has this:

  /* Define to 1 if you have the `umask' function. */
  #if defined(__MINGW32__) || defined(__TINYC__)
  # define HAVE_UMASK 1
  #endif

and makeint.h says around line 315:

  #if !defined(HAVE_UMASK)
  typedef int mode_t;
  extern mode_t umask (mode_t);
  #endif

So the typedef in makeint.h should have been excluded in the MinGW
build.  How come it wasn't in your case?  Did MinGW64 stop defining
__MINGW32__ or something?  Or was makeint.h included before config.h?

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

[Dmitry Goncharov via Bug reports and discussion for GNU make]

Wanted to test that windows this still works with this change.

So, i followed instructions in README.W32.
Installed mingw gcc. Copied config.h.W32.template to config.h.w32.
Ran build_W32.bat gcc. It fails with

C:\dgoncharov\make>gcc -mthreads -Wall -std=gnu99 -gdwarf-2 -g3 -O2 -DMAKE_MAINT
AINER_MODE -I.\GccRel/src -I./src -I.\GccRel/lib -I./lib -I./src/w32/include -DW
INDOWS32 -DHAVE_CONFIG_H  -o .\GccRel/src/ar.o -c src/ar.c
In file included from src/ar.c:18:
src/makeint.h:314:13: error: conflicting types for 'mode_t'
 typedef int mode_t;
 ^~
In file included from src/makeint.h:86,
 from src/ar.c:18:
c:/Program Files (x86)/mingw-w64/i686-8.1.0-posix-dwarf-rt_v6-rev0/mingw32/i686-
w64-mingw32/include/sys/types.h:77:17: note: previous declaration of 'mode_t' wa
s here  typedef _mode_t mode_t;

Both make and mingw declare mode_t.
More errors like this follow.

Does anyone have a ready to use config.h.W32?


regards, Dmitry

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

[Dmitry Goncharov]

Follow-up Comment #7, bug #60774 (project make):

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.



___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

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

[Dmitry Goncharov via Bug reports and discussion for GNU make]

On Sat, Jul 24, 2021 at 3:02 PM David Boyce  wrote:

> The point is that only /dev/null *should* be shared. Any other file *might* 
> be locked by an unrelated process but that would represent an error condition 
> in which incorrect results should be expected.

i think, it is less likely that an unrelated process locks a make's
private temp file, than a system wide global file.

> I can’t think of a file other than /dev/null which would appropriately be 
> shared with unrelated processes in a “w” (write) condition.

/dev/zero, /dev/lp0, /dev/lp1, /dev/pts, etc.
i recognize the simplicity of using stdout. However, i also dislike
adding pieces of code for a set of special files. The user will always
find a way to screw it.

Also, make is portable. What about those systems which do not have
inode/devices?

regards, Dmitry

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

[David Boyce]

> That would help this specific case indeed...

The point is that only /dev/null *should* be shared. Any other file *might* be 
locked by an unrelated process but that would represent an error condition in 
which incorrect results should be expected. I can’t think of a file other than 
/dev/null which would appropriately be shared with unrelated processes in a “w” 
(write) condition. 

> On Jul 24, 2021, at 2:15 PM, Dmitry Goncharov  wrote:
> 
> That would help this specific case indeed.

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

[Eli Zaretskii]

Follow-up Comment #6, bug #60774 (project make):

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.


___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

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

[Dmitry Goncharov via Bug reports and discussion for GNU make]

On Saturday, July 24, 2021, David Boyce  wrote:

> Wouldn’t a less intrusive solution be to check the inode/device of stdout
> and if it’s the same as that of /dev/null just forego locking?


That would help this specific case indeed.
What about a case when make's stdout is redirected to another, not dev
null, file? Any file could be locked.

Another case is when a child's stdout is redirected to a file different
from parent.

Regards, Dmitry

>
> > On Jul 24, 2021, at 11:26 AM, Eli Zaretskii 
> wrote:
> >
> > Follow-up Comment #5, bug #60774 (project make):
> >
> > 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?
> >
> >
> >___
> >
> > Reply to this item at:
> >
> >  
> >
> > ___
> >  Message sent via Savannah
> >  https://savannah.gnu.org/
> >
> >
>

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

[Dmitry Goncharov via Bug reports and discussion for GNU make]

On Saturday, July 24, 2021, Eli Zaretskii  wrote:

> Follow-up Comment #5, bug #60774 (project make):
>
> 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.


True.

>
> 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?


The windows code, which passes down the semaphore to children is reused to
pass down the fd on unix. If we leave it alone, we'd have to duplicate the
mechanism on unix.


Regards, Dmitry

> ___
>
> Reply to this item at:
>
>   
>
> ___
>   Message sent via Savannah
>   https://savannah.gnu.org/
>
>

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

[David Boyce]

Wouldn’t a less intrusive solution be to check the inode/device of stdout and 
if it’s the same as that of /dev/null just forego locking?

> On Jul 24, 2021, at 11:26 AM, Eli Zaretskii  wrote:
> 
> Follow-up Comment #5, bug #60774 (project make):
> 
> 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?
> 
> 
>___
> 
> Reply to this item at:
> 
>  
> 
> ___
>  Message sent via Savannah
>  https://savannah.gnu.org/
> 
> 

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

[Eli Zaretskii]

Follow-up Comment #5, bug #60774 (project make):

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?


___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

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

[Dmitry Goncharov]

Follow-up Comment #4, bug #60774 (project make):

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

___

Reply to this item at:

  

___
  Сообщение отправлено по Savannah
  https://savannah.gnu.org/

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

[Dmitry Goncharov]

Follow-up Comment #3, bug #60774 (project make):

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.

___

Reply to this item at:

  

___
  Сообщение отправлено по Savannah
  https://savannah.gnu.org/

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

[Dmitry Goncharov]

Follow-up Comment #2, bug #60774 (project make):

Thanks for your report, Mike.

make uses file locking on stdout to synchronize output.

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

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.

___

Reply to this item at:

  

___
  Сообщение отправлено по Savannah
  https://savannah.gnu.org/

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

[Mike Frysinger]

Follow-up Comment #1, bug #60774 (project make):

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.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

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

[Mike Frysinger]

URL:
  

 Summary: make hangs on fcntl lock when using -O and stdout is
/dev/null
 Project: make
Submitted by: vapier
Submitted on: Fri 11 Jun 2021 06:24:02 PM EDT
Severity: 3 - Normal
  Item Group: None
  Status: None
 Privacy: Public
 Assigned to: None
 Open/Closed: Open
 Discussion Lock: Any
   Component Version: None
Operating System: None
   Fixed Release: None
   Triage Status: None

___

Details:

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  0x7f10555c9643 in fcntl64 () from /lib64/libc.so.6
#1  0x002225ff in acquire_semaphore () at output.c:267
#2  output_dump (out=out@entry=0x13a9744) at output.c:368
#3  0x0021b43a in reap_children (block=block@entry=0x1, err=, err@entry=0x0) at job.c:937
#4  0x00227c99 in update_goal_chain (goaldeps=) at
remake.c:112
#5  0x0021fa1b in main (argc=0x5, argv=0x7ffdd42b3478,
envp=0x7ffdd42b34a8) at main.c:2558

(gdb) f 1
#1  0x002225ff in acquire_semaphore () at output.c:267
267   if (fcntl (sync_handle, F_SETLKW, ) != -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, ) != -1)
268 return 
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.



___

File Attachments:


---
Date: Fri 11 Jun 2021 06:24:02 PM EDT  Name: log.xz  Size: 443KiB   By: vapier



___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/