public inbox for goredo-devel@lists.cypherpunks.ru
Atom feed
From: Sergey Matveev <stargrave@stargrave•org>
To: goredo-devel@lists.cypherpunks.ru
Subject: Re: File descriptor leak
Date: Sun, 8 May 2022 14:42:55 +0300 [thread overview]
Message-ID: <Ynesv6Sy83pVQ5x4@stargrave.org> (raw)
In-Reply-To: <6a75db3d-7d4e-6921-db74-50bd4b592108@acha.ninja>
[-- Attachment #1: Type: text/plain, Size: 10324 bytes --]
Greetings!
*** Andrew Chambers [2022-05-08 16:44]:
>I am fairly sure this is a
>file descriptor leak in goredo, as I don't have any problems with other redo
>implementations.
As spacefrogg already answered, goredo opens lock files for all targets
came from the command line. I tried you default.do with musl and saw:
PID COMM FD T V FLAGS REF OFFSET PRO NAME
11263 goredo text v r r------- - - - /home/stargrave/work/goredo/goredo
11263 goredo ctty v c rw------ - - - /dev/pts/22
11263 goredo cwd v d r------- - - - /tmp/musl-1.2.3
11263 goredo root v d r------- - - - /
11263 goredo 0 v c r------- 2 0 - /dev/null
11263 goredo 1 p - rw------ 4 0 - -
11263 goredo 2 p - rw------ 4 0 - -
11263 goredo 3 v r -w------ 1 0 - /tmp/musl-1.2.3/src/select/.redo/poll.o.lock
11263 goredo 4 v r rw------ 3 0 - -
11263 goredo 5 p - rw------ 28 0 - -
11263 goredo 6 p - rw------ 31 0 - -
11263 goredo 7 p - rw------ 28 0 - -
11263 goredo 8 v r -wa----- 3 0 - /tmp/musl-1.2.3/.redo/.redo.libc.a.rec.8d2431ca
11263 goredo 9 v r rw------ 21 53097 - -
11263 goredo 10 v r rw------ 22 0 - -
11263 goredo 11 v r -w------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo/timerfd.o.lock
11263 goredo 12 v r -w------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo/unshare.o.lock
11263 goredo 13 v r rw------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo.prctl.o.9b587891
11263 goredo 14 v r -w------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo/utimes.o.lock
11263 goredo 15 v r -w------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo/vhangup.o.lock
11263 goredo 16 v r -wa----- 3 0 - /tmp/musl-1.2.3/src/linux/.redo/.redo.prctl.o.rec.9b55ee9a
11263 goredo 17 k - rw------ 2 0 - -
11263 goredo 18 p - rw---n-- 2 0 - -
11263 goredo 19 p - rw---n-- 1 0 - -
11263 goredo 20 p - rw---n-- 3 0 - -
11263 goredo 21 v r rw------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo.name_to_handle_at.o.9b4468e0
11263 goredo 22 v r -wa----- 3 0 - /tmp/musl-1.2.3/src/linux/.redo/.redo.name_to_handle_at.o.rec.9b41bf26
11263 goredo 23 v r rw------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo.memfd_create.o.9b341573
11263 goredo 24 v r rw------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo.pivot_root.o.9b50a895
11263 goredo 25 v r -wa----- 3 59585 - /tmp/musl-1.2.3/src/linux/.redo/.redo.memfd_create.o.rec.9b315eb7
11263 goredo 26 p - rw---n-- 3 0 - -
11263 goredo 27 v r -wa----- 3 0 - /tmp/musl-1.2.3/src/linux/.redo/.redo.pivot_root.o.rec.9b4e1c31
11263 goredo 28 v r rw------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo.mount.o.9b404696
11263 goredo 29 v r -wa----- 3 56373 - /tmp/musl-1.2.3/src/linux/.redo/.redo.mount.o.rec.9b3dad33
11263 goredo 30 p - rw---n-- 3 0 - -
11263 goredo 31 p - rw---n-- 3 0 - -
11263 goredo 32 v r rw------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo.personality.o.9b4cb7c2
11263 goredo 33 v r -wa----- 3 0 - /tmp/musl-1.2.3/src/linux/.redo/.redo.personality.o.rec.9b4a0ca2
11263 goredo 34 p - rw---n-- 3 0 - -
11263 goredo 35 p - rw---n-- 3 0 - -
11263 goredo 36 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/signgam.o.lock
11263 goredo 37 v r rw------ 2 0 - /tmp/musl-1.2.3/src/linux/.redo.ppoll.o.9b5491f0
11263 goredo 38 v r -wa----- 2 0 - /tmp/musl-1.2.3/src/linux/.redo/.redo.ppoll.o.rec.9b520a93
11263 goredo 39 v r rw------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo.gettid.o.9b0f6737
11263 goredo 40 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/scalblnl.o.lock
11263 goredo 41 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/significand.o.lock
11263 goredo 42 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/significandf.o.lock
11263 goredo 43 v r -w------ 1 0 - /tmp/musl-1.2.3/src/select/.redo/pselect.o.lock
11263 goredo 44 p - rw---n-- 3 0 - -
11263 goredo 45 p - rw------ 7 0 - -
11263 goredo 46 v r -w------ 1 0 - /tmp/musl-1.2.3/src/locale/.redo/catclose.o.lock
11263 goredo 47 v c r------- 3 0 - /dev/null
11263 goredo 48 p - rw------ 1 0 - -
11263 goredo 49 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/sinf.o.lock
11263 goredo 50 v r -wa----- 3 59573 - /tmp/musl-1.2.3/src/linux/.redo/.redo.gettid.o.rec.9b0cdcb5
11263 goredo 51 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/sin.o.lock
11263 goredo 52 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/cbrt.o.lock
11263 goredo 53 v r -w------ 1 0 - /tmp/musl-1.2.3/src/locale/.redo/catgets.o.lock
11263 goredo 54 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/sincos.o.lock
11263 goredo 55 v r -w------ 1 0 - /tmp/musl-1.2.3/src/locale/.redo/c_locale.o.lock
11263 goredo 56 v r -w------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo/sethostname.o.lock
11263 goredo 57 v r -w------ 1 0 - /tmp/musl-1.2.3/src/misc/.redo/setdomainname.o.lock
11263 goredo 58 p - rw---n-- 3 0 - -
11263 goredo 61 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/asinhl.o.lock
11263 goredo 62 v r -w------ 1 0 - /tmp/musl-1.2.3/src/linux/.redo/wait4.o.lock
11263 goredo 63 v r -w------ 1 0 - /tmp/musl-1.2.3/src/stdio/.redo/fseek.o.lock
11263 goredo 64 v r -w------ 1 0 - /tmp/musl-1.2.3/src/mman/.redo/mmap.o.lock
11263 goredo 65 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/__invtrigl.o.lock
11263 goredo 66 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/sinh.o.lock
11263 goredo 67 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/cbrtf.o.lock
11263 goredo 68 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/atan2f.o.lock
11263 goredo 69 v r -w------ 1 0 - /tmp/musl-1.2.3/src/math/.redo/cbrtl.o.lock
[more than thousand of opened lock files]
and it was run with more than a thousand of targets indeed.
spacefrogg advised to use xargs to limit number of arguments passed.
That will work and I like that. Making artificial "xargs"-like limiter
inside goredo leads to the question: how many arguments should be
processed at once? Of course that could be configurable, but that is an
additional burden for the end-user, completely the same as adding xargs.
For example github.com/leahneukirchen/redo-c does not have the problems
with many opened lockfiles, because it takes jobserver's token before it
opens those lockfiles. I explicitly decided to take jobserver's token
after we really decided that target must be done and run. Goredo opens
lockfiles, checks various metainformation, makes some preparations and
temporary files and only after that it acquires jobserver's token to run
the do-file itself. I explicitly wanted to acquire token as late as
possible.
But that leads to possibility of many simultaneously opened (maximally
prepared target before acquiring the jobserver's token and running the
command itself), that, as I can see, brings more complications and
distractions to the end user.
So let's acquire jobserver's token (that already acts as a limiter for
number of jobs simultaneously running) before we open the lockfile. That
will solve the problem of many opened lockfiles. If you pass unlimited
number of jobs (-j 0), then of course it will open as many files as it
wish.
There is a drawback: we can acquire jobserver's token, open the
lockfile, process metadata and see that actually that target was already
done, so we return the jobserver's token back -- it was useless and no
do-file was actually run. But I did not notice any performance
differences in practice, because all of that seems to be anyway pretty
fast enough and not deserving the attention.
So with 1.25.0 that kind of problems should pass away. I do not like
that, but seems that is pretty acceptable compromise, with only a minor
changes in the code.
I also overviewed my code again and found several places where file
descriptors were not closed. But all of them were only related to failed
situations or supplementary helper commands.
By the way, just a remark, your default.do contains "set -o pipefail",
that does not exist in POSIX shell. For example FreeBSD's /bin/sh tells:
set: Illegal option -o pipefail
--
Sergey Matveev (http://www.stargrave.org/)
OpenPGP: CF60 E89A 5923 1E76 E263 6422 AE1A 8109 E498 57EF
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2022-05-08 11:42 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-08 4:44 File descriptor leak Andrew Chambers
2022-05-08 8:22 ` goredo
2022-05-08 9:12 ` Andrew Chambers
2022-05-08 11:21 ` goredo
2022-05-08 11:42 ` Sergey Matveev [this message]
2022-05-09 0:31 ` Andrew Chambers
2022-05-09 7:10 ` Sergey Matveev