Re: [nmh-workers] closefds() _before_ fork?

2019-04-25 Thread Ken Hornstein
>Basically, it opens a file and attempts to obtain an exclusive lock the >file descriptor for that file. If/when it successfully obtains the >exclusive lock it then uses exec() to execute a new process, and that >new process inherits the open file descriptor. As long as that process

Re: [nmh-workers] closefds() _before_ fork?

2019-04-24 Thread Andy Bradford
Thus said Ken Hornstein on Tue, 23 Apr 2019 19:35:11 -0400: > >setlock /tmp/mit.lock inc +MyIncTmp ... > > Well, my reading of the setlock man page (assuming this is the FreeBSD > one) is that it is not depending on a _descriptor_ to be passed down > (I don't even know how that would work),

Re: [nmh-workers] closefds() _before_ fork?

2019-04-24 Thread Ralph Corderoy
Hi Ken, > > > they called getdtablesize() on Linux, which it seems returns a > > > smaller number than getrlimit(). > > > > That's surprising. I thought getdtablesize() was effectively > > > > return getrlimit(RLIMIT_NOFILE, ) < 0 ? OPEN_MAX : ru.rlim_cur; > > Hey, I don't make the news, I

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Robert Elz
Date:Tue, 23 Apr 2019 19:35:11 -0400 From:Ken Hornstein Message-ID: <20190423233511.ea88b143...@pb-smtp1.pobox.com> | There are tons of examples like this in nmh; plenty of child processes | are created with descriptors open. It is worth remembering that MH was

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ken Hornstein
>> to that end, i propose that we treat any open descriptor N>2 at the >> time of an exec() to be a bug, which is to be fixed by setting >> O_CLOEXEC at time of creation. > >What impact will such a decision have on tools like setlock which open a >filedescriptor, obtain an exclusive

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Valdis Klētnieks
On Wed, 24 Apr 2019 03:46:04 +0700, Robert Elz said: > fd is also made to be fd 3. > > If it is good enough to be stdin, it is good enough to be 3 as well, > if there is (or once was) some reason this is important. Derp. -ENOCAFFEINE :) -- nmh-workers

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Andy Bradford
Thus said Paul Vixie on Tue, 23 Apr 2019 05:59:05 -0700: > to that end, i propose that we treat any open descriptor N>2 at the > time of an exec() to be a bug, which is to be fixed by setting > O_CLOEXEC at time of creation. What impact will such a decision have on tools like setlock

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Robert Elz
Date:Tue, 23 Apr 2019 14:39:48 -0400 From:"Valdis Kl=?utf-8?Q?=c4=93?=tnieks" Message-ID: <1.1556044788@turing-police> | The point is that the fragment of code doesn't actually *know* "it happened | to be what is wanted on stdin". For all this code knows,

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Robert Elz
Date:Tue, 23 Apr 2019 11:15:05 -0400 From:"Valdis Kl=?utf-8?Q?=c4=93?=tnieks" Message-ID: <4344.1556032505@turing-police> | The *real* WTF there is that the code doesn't actually know if fd 3 is | known to be open on a file that should, or if it's just a stray

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ken Hornstein
>Ken Hornstein writes: >> I realize that dup2() clears the FD_CLOEXEC flag on the new file >> descriptor so the "normal" case of an opened file being dup2() down to 0 >> would work correctly, but the wrinkle is that it does NOT if the old and >> new file descriptor are the same. That is

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Tom Lane
Ken Hornstein writes: > I realize that dup2() clears the FD_CLOEXEC flag on the new file > descriptor so the "normal" case of an opened file being dup2() down to 0 > would work correctly, but the wrinkle is that it does NOT if the old and > new file descriptor are the same. That is admittedly

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ken Hornstein
>> I realize that dup2() clears the FD_CLOEXEC flag on the new file >> descriptor so the "normal" case of an opened file being dup2() down to >> 0 would work correctly, but the wrinkle is that it does NOT if the old >> and new file descriptor are the same. That is admittedly unlikely, >> but it

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ralph Corderoy
Hi Ken, > I realize that dup2() clears the FD_CLOEXEC flag on the new file > descriptor so the "normal" case of an opened file being dup2() down to > 0 would work correctly, but the wrinkle is that it does NOT if the old > and new file descriptor are the same. That is admittedly unlikely, > but

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ken Hornstein
>> It's not the only one, e.g. docs/historical/mh-6.8.5/uip/post.c has >> >> 2622 if (fd != 0) >> 2623 (void) dup2 (fd, 0); >> 2624 (void) freopen ("/dev/null", "w", stdout); >> 2625 (void) freopen ("/dev/null", "w", stderr); >>

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ken Hornstein
>> I agree with the general principle that if we open it, we track it, and >> then close it so it doesn't reach the child, typically with O_CLOEXEC or >> FD_CLOEXEC. ... > >to that end, i propose that we treat any open descriptor N>2 at the time >of an exec() to be a bug, which is to be fixed by

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Valdis Klētnieks
On Tue, 23 Apr 2019 13:35:44 +0100, Ralph Corderoy said: > It's not the only one, e.g. docs/historical/mh-6.8.5/uip/post.c has > > 2622 if (fd != 0) > 2623 (void) dup2 (fd, 0); > 2624 (void) freopen ("/dev/null", "w", stdout); > 2625

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ken Hornstein
>> they called getdtablesize() on Linux, which it seems returns a smaller >> number than getrlimit(). > >That's surprising. I thought getdtablesize() was effectively > > return getrlimit(RLIMIT_NOFILE, ) < 0 ? OPEN_MAX : ru.rlim_cur; Hey, I don't make the news, I just report it. If you look

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ralph Corderoy
Hi Ken, > they called getdtablesize() on Linux, which it seems returns a smaller > number than getrlimit(). That's surprising. I thought getdtablesize() was effectively return getrlimit(RLIMIT_NOFILE, ) < 0 ? OPEN_MAX : ru.rlim_cur; That seems to match /lib/libc.a here, if I squint a

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ken Hornstein
>I agree with the general principle that if we open it, we track it, and >then close it so it doesn't reach the child, typically with O_CLOEXEC or >FD_CLOEXEC. Calling close(2) lots of times based on getdtablesize(3) >can take a while. screen(1) had a bug recently where it was taking ages

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Paul Vixie
Ralph Corderoy wrote on 2019-04-23 05:10: ... I agree with the general principle that if we open it, we track it, and then close it so it doesn't reach the child, typically with O_CLOEXEC or FD_CLOEXEC. ... to that end, i propose that we treat any open descriptor N>2 at the time of an

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ralph Corderoy
Hi Ken, > but ALSO uses dup2() to make an additional copy of the input file > descriptor to descriptor 3 (!). Does anyone know why? It looks like > it has always done this. It's not the only one, e.g. docs/historical/mh-6.8.5/uip/post.c has 2622 if (fd != 0) 2623

Re: [nmh-workers] closefds() _before_ fork?

2019-04-23 Thread Ralph Corderoy
Hi, David wrote: > Ken wrote: > > I suggest we simply remove closefds() completely. ... > Ralph's suggestion of looking for only open fds addresses the > suboptimality of closefds(), though only for systems that use /proc. That was just meant as a temporary debugging aid to see what was sneaking

Re: [nmh-workers] closefds() _before_ fork?

2019-04-22 Thread Ken Hornstein
>David Levine writes: >> Ken wrote: >>> I suggest we simply remove closefds() completely. > >> Great idea! > >Makes sense from here. You could back-fill use of CLOEXEC anywhere >it seemed important. Alright, done. In the "weird things I discovered while doing this" ... I saw in slocal that

Re: [nmh-workers] closefds() _before_ fork?

2019-04-22 Thread Tom Lane
David Levine writes: > Ken wrote: >> I suggest we simply remove closefds() completely. > Great idea! Makes sense from here. You could back-fill use of CLOEXEC anywhere it seemed important. regards, tom lane -- nmh-workers

Re: [nmh-workers] closefds() _before_ fork?

2019-04-22 Thread David Levine
Ken wrote: > I suggest we simply remove closefds() completely. Great idea! Ralph's suggestion of looking for only open fds addresses the suboptimality of closefds(), though only for systems that use /proc. I'd prefer to see the application take better care of its resources, but that's much

Re: [nmh-workers] closefds() _before_ fork?

2019-04-22 Thread Ken Hornstein
>I understand why functions like closefds() are used right before exec(), >but in this case a fair amonut of stuff happens before exec is called, >and it's way before fork() is eventually called as well. The other uses >of closefds() are after a fork() occurs, or right before exec() happens >(see

Re: [nmh-workers] closefds() _before_ fork?

2019-04-22 Thread David Levine
Ken wrote: > Can anyone think of a reason that for this specific > case closefds() should NOT be moved from whatnowbr.c (and send.c) into > sendsbr.c, and just in the child process? That looks like a good change. The callers of sendsbr() shouldn't be concerned with its fork/exec. Those two

Re: [nmh-workers] closefds() _before_ fork?

2019-04-22 Thread Ralph Corderoy
Hi Ken, > Can anyone think of a reason that for this specific case closefds() > should NOT be moved from whatnowbr.c (and send.c) into sendsbr.c, and > just in the child process? IT does seem odd to have it in the parent rather than just the child. The only thing I can think of is if it's

[nmh-workers] closefds() _before_ fork?

2019-04-22 Thread Ken Hornstein
I've been having a problem when occasionally at a What now? prompt when I use the "send" command the process will get killed with a -9 out of nowhere (this is on MacOS X High Sierra). I finally realized that the same problem is causing the test suite to fail for the dist test, so I decided to