9fans - fans of the OS Plan 9 from Bell Labs
 help / color / mirror / Atom feed
* [9fans] file descriptor leak
@ 2016-02-16 15:52 arisawa
  2016-02-16 15:56 ` Jacob Todd
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: arisawa @ 2016-02-16 15:52 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

Hello,

I have observed warning messages from dns server:
dns 30792: warning process exceeded 100 file descriptors
dns 30888: warning process exceeded 200 file descriptors
…

probably the file descriptor leak comes from dnresolve.c

udpquery(Query *qp, char *mntpt, int depth, int patient, int inns)
{
	…
			msg = system(open("/dev/null", ORDWR), "outside");
	…
}

char *
system(int fd, char *cmd)
{
	int pid, p, i;
	static Waitmsg msg;

	if((pid = fork()) == -1)
		sysfatal("fork failed: %r");
	else if(pid == 0){
		dup(fd, 0);
		close(fd);
		for (i = 3; i < 200; i++)
			close(i);		/* don't leak fds */
		execl("/bin/rc", "rc", "-c", cmd, nil);
		sysfatal("exec rc: %r");
	}
	for(p = waitpid(); p >= 0; p = waitpid())
		if(p == pid)
			return msg.msg;
	return "lost child";
}

fd is lost if pid > 0

my server is running on 9front. however both 9atom and bell-labs use same routine.

Kenji Arisawa





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 15:52 [9fans] file descriptor leak arisawa
@ 2016-02-16 15:56 ` Jacob Todd
  2016-02-16 16:42   ` lucio
  2016-02-16 18:01 ` cinap_lenrek
  2016-02-16 22:24 ` Charles Forsyth
  2 siblings, 1 reply; 13+ messages in thread
From: Jacob Todd @ 2016-02-16 15:56 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

[-- Attachment #1: Type: text/plain, Size: 1406 bytes --]

I've had this problem too, I have yet to resolve it.
On Feb 16, 2016 10:54 AM, "arisawa" <arisawa@ar.aichi-u.ac.jp> wrote:

> Hello,
>
> I have observed warning messages from dns server:
> dns 30792: warning process exceeded 100 file descriptors
> dns 30888: warning process exceeded 200 file descriptors
> …
>
> probably the file descriptor leak comes from dnresolve.c
>
> udpquery(Query *qp, char *mntpt, int depth, int patient, int inns)
> {
>         …
>                         msg = system(open("/dev/null", ORDWR), "outside");
>         …
> }
>
> char *
> system(int fd, char *cmd)
> {
>         int pid, p, i;
>         static Waitmsg msg;
>
>         if((pid = fork()) == -1)
>                 sysfatal("fork failed: %r");
>         else if(pid == 0){
>                 dup(fd, 0);
>                 close(fd);
>                 for (i = 3; i < 200; i++)
>                         close(i);               /* don't leak fds */
>                 execl("/bin/rc", "rc", "-c", cmd, nil);
>                 sysfatal("exec rc: %r");
>         }
>         for(p = waitpid(); p >= 0; p = waitpid())
>                 if(p == pid)
>                         return msg.msg;
>         return "lost child";
> }
>
> fd is lost if pid > 0
>
> my server is running on 9front. however both 9atom and bell-labs use same
> routine.
>
> Kenji Arisawa
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 1982 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 15:56 ` Jacob Todd
@ 2016-02-16 16:42   ` lucio
  2016-02-16 17:05     ` Jacob Todd
  2016-02-16 17:17     ` Charles Forsyth
  0 siblings, 2 replies; 13+ messages in thread
From: lucio @ 2016-02-16 16:42 UTC (permalink / raw)
  To: 9fans

> I've had this problem too, I have yet to resolve it.

You could start by tossing the very first and totally superfluous
"else".  to simplify things.

Then, it would be tempting to take the

	dup(fd,0); close(fd);

out to before the if(pid==0)...

Well spotted, Kenji.  Let's hope my shot in the dark is half as good :-)

Lucio.




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 16:42   ` lucio
@ 2016-02-16 17:05     ` Jacob Todd
  2016-02-16 17:17     ` Charles Forsyth
  1 sibling, 0 replies; 13+ messages in thread
From: Jacob Todd @ 2016-02-16 17:05 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

[-- Attachment #1: Type: text/plain, Size: 30 bytes --]

what's your fucking problem?

[-- Attachment #2: Type: text/html, Size: 51 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 16:42   ` lucio
  2016-02-16 17:05     ` Jacob Todd
@ 2016-02-16 17:17     ` Charles Forsyth
  1 sibling, 0 replies; 13+ messages in thread
From: Charles Forsyth @ 2016-02-16 17:17 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

On 16 February 2016 at 16:42, <lucio@proxima.alt.za> wrote:

> Then, it would be tempting to take the
>
>         dup(fd,0); close(fd);
>
> out to before the if(pid==0)...
>

the idea is to have fd (/dev/null in this case) be standard input in the
new process,
so it needs to follow the pid==0 test.

the "outside" command is one that wasn't distributed (I assume it bound a
separate #I interface
on /net.alt and set it up), so the code currently probably doesn't do
anything useful elsewhere.


> probably the file descriptor leak comes from dnresolve.c


you can cat /proc/$dnspid/fd
where dnspid is the process id one or more of the active dns processes,
to see which files are open, after the message appears.
if there are many /dev/null open, that suggests your idea was right.

i think you're right that it leaks an fd to /dev/null in that system call,
so
it should instead open /dev/null separately and assign fd before the call
and close it afterwards.

even so, i wonder if that's really what's happening in every case of "more
than N fds", because
the call to outside is only needed in the case that the udp under /net.alt
is being
used and an open there has failed. still, looking at the /proc/N/fd file
should help decide that.

[-- Attachment #2: Type: text/html, Size: 2588 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 15:52 [9fans] file descriptor leak arisawa
  2016-02-16 15:56 ` Jacob Todd
@ 2016-02-16 18:01 ` cinap_lenrek
  2016-02-16 21:05   ` erik quanstrom
  2016-02-16 21:16   ` Charles Forsyth
  2016-02-16 22:24 ` Charles Forsyth
  2 siblings, 2 replies; 13+ messages in thread
From: cinap_lenrek @ 2016-02-16 18:01 UTC (permalink / raw)
  To: 9fans

its worse. the static msg.msg is useless, its never set anywhere. the
interface of system() makes no sense. its only used for running "outside"
and the parent proc doesnt need the fd to /dev/null, it could as well just
open it in the child like:

close(0); open("/dev/null", OREAD);

the caller goes at some lengths queueing processes waiting for the remount
to complete, but does it in two levels. thers a sleep and a qlock.
limitation on the number of processes that can be queued on a qlock()?

also, when fork() returns -1, it kills the parent.

the whole thing about remounting /net.alt seems wrong. it probably fixed
some issue at the labs with /net.alt being imported from some other machine
that kept rebooting or something.

after spending 5 minutes writing the code fixing all these issues mentiond
above, i'll just throw it all away and delete the whole remounting logic
for /net.alt in 9front.

--
cinap



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 18:01 ` cinap_lenrek
@ 2016-02-16 21:05   ` erik quanstrom
  2016-02-16 21:16   ` Charles Forsyth
  1 sibling, 0 replies; 13+ messages in thread
From: erik quanstrom @ 2016-02-16 21:05 UTC (permalink / raw)
  To: 9fans

> limitation on the number of processes that can be queued on a qlock()?

in user space, yes.

- erik



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 18:01 ` cinap_lenrek
  2016-02-16 21:05   ` erik quanstrom
@ 2016-02-16 21:16   ` Charles Forsyth
  2016-02-17  2:19     ` cinap_lenrek
  2016-02-22  5:18     ` erik quanstrom
  1 sibling, 2 replies; 13+ messages in thread
From: Charles Forsyth @ 2016-02-16 21:16 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]

On 16 February 2016 at 18:01, <cinap_lenrek@felloff.net> wrote:

> and the parent proc doesnt need the fd to /dev/null, it could as well just
> open it in the child like:
>
> close(0); open("/dev/null", OREAD);
>
>
There's no harm in making and using a more general function, even in a
specific way, so that part's ok.
The caller just needs to play its part properly.

after spending 5 minutes writing the code fixing all these issues mentiond
> above, i'll just throw it all away and delete the whole remounting logic
> for /net.alt in 9front.


It's often better to use the Erlang fail-fast ("just fail") and restart
approach for persistent services.

More important would be to look at /proc/N/fd on a failing system.
I've a feeling that the system/outside stuff isn't actually the problem,
since I've seen the diagnostic on a system that wasn't using /net.alt.
In that case, the problem (as I remember it) was that an Internet link
further on was down,
so no messages got through to remote DNS, and file descriptors were
building up in slave processes
waiting for replies on /net/udp. Once the link was up, it went back to
normal.

[-- Attachment #2: Type: text/html, Size: 2339 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 15:52 [9fans] file descriptor leak arisawa
  2016-02-16 15:56 ` Jacob Todd
  2016-02-16 18:01 ` cinap_lenrek
@ 2016-02-16 22:24 ` Charles Forsyth
  2016-02-17  1:13   ` arisawa
  2 siblings, 1 reply; 13+ messages in thread
From: Charles Forsyth @ 2016-02-16 22:24 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]

On 16 February 2016 at 15:52, arisawa <arisawa@ar.aichi-u.ac.jp> wrote:

>
> I have observed warning messages from dns server:
> dns 30792: warning process exceeded 100 file descriptors
> dns 30888: warning process exceeded 200 file descriptors
>

It's worth noting that this message doesn't necessarily mean you've got a
file descriptor leak.
It might, but at the start it just means that a process (or process group
sharing a file descriptor group) has many file descriptors open.
That could happen if a server with many clients has a file descriptor per
client and then client requests open some more.

ndb/dns in particular will try user-level requests in parallel, and those
in turn can lead to several concurrent
queries to various name servers at a given level (root itself has 13). Web
browser clients will fetch page
elements concurrently. That's why it's useful to check the /proc/N/fd file
to try to see what they are.
(Not just for ndb/dns, but for other applications that provoke the message.)
Arguably, if you're using the system in a real, Internet-facing
application, the warning message might
be obsolete, compared to the time when even 100 clients was a big network.

[-- Attachment #2: Type: text/html, Size: 1791 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 22:24 ` Charles Forsyth
@ 2016-02-17  1:13   ` arisawa
  2016-02-17  1:22     ` cinap_lenrek
  0 siblings, 1 reply; 13+ messages in thread
From: arisawa @ 2016-02-17  1:13 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

the logic (code? usage?) of system(int fd, char *cmd) is ugly.
thanks for fixing, cinap.

charies may be speaking right point, because the dns server is running only for my home use.
only my family uses the dns service. my home network is very simple.I have never used /net.alt for that.

the dns had been running on file server and sometimes (once a month or so) failed into trouble.
a few weeks ago, I separated it from file server so that I can see the trouble really comes from dns.

the contents of /proc/N/fs are shown below.
it seems /dev/null is not the criminal as charies suggested.
how to find fd leakage?

thanks in advance


io% ps|grep dns
arisawa         248    0:00   0:00      348K Await    dns
arisawa         249    0:00   0:00    14584K Pread    dns
arisawa         250    0:12   0:12    14576K Pread    dns
io% cat /proc/^(248 249 250)^/fd
/usr/arisawa
  0 r  M    8 (0000000000000005 0 00)  8192        1 /dev/cons
  1 w  c    0 (0000000000000002 0 00)     0        1 #c/cons
  2 w  c    0 (0000000000000002 0 00)     0      193 #c/cons
  3 r  c    0 (000000000000000f 0 00)     0        4 /dev/random
  4 w  M   22 (000000000000ce7e 54050 40)  8192  1098023 /sys/log/dns
  5 r  c    0 (0000000000000001 0 00)     0  6873616 /dev/bintime
  6 r  M   22 (000000000000929e 3098 00)  8192    10949 /lib/ndb/local
  7 r  M   22 (000000000000929b 47 00)  8192    10242 /lib/ndb/common
  8 r  I    0 (0000000000000004 3 00)     0      127 /net/ndb
  9 rw |    0 (0000000000000fc1 0 00) 65536     3954 #|/data
 11 w  s    0 (0000000000000009 0 00)     0        2 #s/dns
/usr/arisawa
  0 r  M    8 (0000000000000005 0 00)  8192        1 /dev/cons
  1 w  c    0 (0000000000000002 0 00)     0        1 #c/cons
  2 w  c    0 (0000000000000002 0 00)     0      193 #c/cons
  3 r  c    0 (000000000000000f 0 00)     0        4 /dev/random
  4 w  M   22 (000000000000ce7e 54050 40)  8192  1098023 /sys/log/dns
  5 r  c    0 (0000000000000001 0 00)     0  6873816 /dev/bintime
  6 r  M   22 (000000000000929e 3098 00)  8192    10949 /lib/ndb/local
  7 r  M   22 (000000000000929b 47 00)  8192    10242 /lib/ndb/common
  8 r  I    0 (0000000000000004 3 00)     0      127 /net/ndb
  9 rw |    0 (0000000000000fc1 0 00) 65536     3954 #|/data
 11 w  s    0 (0000000000000009 0 00)     0        2 #s/dns
 12 rw I    0 (000000000002002d 0 00)     0     3362 /net/udp/1/data
/usr/arisawa
  0 r  M    8 (0000000000000005 0 00)  8192        1 /dev/cons
  1 w  c    0 (0000000000000002 0 00)     0        1 #c/cons
  2 w  c    0 (0000000000000002 0 00)     0      193 #c/cons
  3 r  c    0 (000000000000000f 0 00)     0        4 /dev/random
  4 w  M   22 (000000000000ce7e 54050 40)  8192  1098023 /sys/log/dns
  5 r  c    0 (0000000000000001 0 00)     0  6873824 /dev/bintime
  6 r  M   22 (000000000000929e 3098 00)  8192    10949 /lib/ndb/local
  7 r  M   22 (000000000000929b 47 00)  8192    10242 /lib/ndb/common
  8 r  I    0 (0000000000000004 3 00)     0      127 /net/ndb
  9 rw |    0 (0000000000000fc1 0 00) 65536     3954 #|/data
 11 w  s    0 (0000000000000009 0 00)     0        2 #s/dns
 12 rw I    0 (000000000002002d 0 00)     0     3362 /net/udp/1/data
io% 


> 2016/02/17 7:24、Charles Forsyth <charles.forsyth@gmail.com> のメール:
> 
> 
> On 16 February 2016 at 15:52, arisawa <arisawa@ar.aichi-u.ac.jp> wrote:
> 
> I have observed warning messages from dns server:
> dns 30792: warning process exceeded 100 file descriptors
> dns 30888: warning process exceeded 200 file descriptors
> 
> It's worth noting that this message doesn't necessarily mean you've got a file descriptor leak.
> It might, but at the start it just means that a process (or process group sharing a file descriptor group) has many file descriptors open.
> That could happen if a server with many clients has a file descriptor per client and then client requests open some more.
> 
> ndb/dns in particular will try user-level requests in parallel, and those in turn can lead to several concurrent
> queries to various name servers at a given level (root itself has 13). Web browser clients will fetch page
> elements concurrently. That's why it's useful to check the /proc/N/fd file to try to see what they are.
> (Not just for ndb/dns, but for other applications that provoke the message.)
> Arguably, if you're using the system in a real, Internet-facing application, the warning message might
> be obsolete, compared to the time when even 100 clients was a big network.




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-17  1:13   ` arisawa
@ 2016-02-17  1:22     ` cinap_lenrek
  0 siblings, 0 replies; 13+ messages in thread
From: cinap_lenrek @ 2016-02-17  1:22 UTC (permalink / raw)
  To: 9fans

looks good. thanks for reporting mr. arisawa.

--
cinap



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 21:16   ` Charles Forsyth
@ 2016-02-17  2:19     ` cinap_lenrek
  2016-02-22  5:18     ` erik quanstrom
  1 sibling, 0 replies; 13+ messages in thread
From: cinap_lenrek @ 2016-02-17  2:19 UTC (permalink / raw)
  To: 9fans

> It's often better to use the Erlang fail-fast ("just fail") and restart
> approach for persistent services.

indeed. ndb/dns also has this "restart" ctl message where it restarts the
server part but keeps the 9p pipe posted. tho that kills all the fid's and
new fids starts as "dummy" user owned file, not a directory, so after restart
/net/dns vanishes.

doing some further experiments, it appears that when i make new fids start
out as directories it works. :-)

ndb/cs also has some logic where it remounts /srv/dns when opening /net/dns
fails.

--
cinap



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9fans] file descriptor leak
  2016-02-16 21:16   ` Charles Forsyth
  2016-02-17  2:19     ` cinap_lenrek
@ 2016-02-22  5:18     ` erik quanstrom
  1 sibling, 0 replies; 13+ messages in thread
From: erik quanstrom @ 2016-02-22  5:18 UTC (permalink / raw)
  To: 9fans

On Tue Feb 16 13:19:29 PST 2016, charles.forsyth@gmail.com wrote:

> On 16 February 2016 at 18:01, <cinap_lenrek@felloff.net> wrote:
>
> > and the parent proc doesnt need the fd to /dev/null, it could as well just
> > open it in the child like:
> >
> > close(0); open("/dev/null", OREAD);
> >
> >
> There's no harm in making and using a more general function, even in a
> specific way, so that part's ok.
> The caller just needs to play its part properly.
>
> after spending 5 minutes writing the code fixing all these issues mentiond
> > above, i'll just throw it all away and delete the whole remounting logic
> > for /net.alt in 9front.
>
>
> It's often better to use the Erlang fail-fast ("just fail") and restart
> approach for persistent services.
>
> More important would be to look at /proc/N/fd on a failing system.
> I've a feeling that the system/outside stuff isn't actually the problem,
> since I've seen the diagnostic on a system that wasn't using /net.alt.
> In that case, the problem (as I remember it) was that an Internet link
> further on was down,
> so no messages got through to remote DNS, and file descriptors were
> building up in slave processes
> waiting for replies on /net/udp. Once the link was up, it went back to
> normal.

we saw this a lot at coraid, but never did catch a smoking-gun process.
i don't recall a perfect correlation to internet down.

- erik



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-02-22  5:18 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-16 15:52 [9fans] file descriptor leak arisawa
2016-02-16 15:56 ` Jacob Todd
2016-02-16 16:42   ` lucio
2016-02-16 17:05     ` Jacob Todd
2016-02-16 17:17     ` Charles Forsyth
2016-02-16 18:01 ` cinap_lenrek
2016-02-16 21:05   ` erik quanstrom
2016-02-16 21:16   ` Charles Forsyth
2016-02-17  2:19     ` cinap_lenrek
2016-02-22  5:18     ` erik quanstrom
2016-02-16 22:24 ` Charles Forsyth
2016-02-17  1:13   ` arisawa
2016-02-17  1:22     ` cinap_lenrek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).