[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

[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

[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/

[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/

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

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

[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

[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.

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

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

[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.
>
>
>
>