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

2021-07-26 Thread 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

2021-07-25 Thread 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

2021-07-25 Thread 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

2021-07-25 Thread 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

2021-07-25 Thread 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

2021-07-25 Thread 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

2021-07-25 Thread 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

2021-07-25 Thread 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

2021-07-25 Thread 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.




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

2021-07-25 Thread 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

2021-07-24 Thread 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



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

2021-07-24 Thread 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

2021-07-24 Thread 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.



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

2021-07-24 Thread 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

2021-07-24 Thread 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

2021-07-24 Thread 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/
> 
>