#302079 s-s-d: Support to set ulimit

Package:
dpkg
Source:
dpkg
Description:
Debian package management system
Submitter:
Carlo Contavalli
Date:
2015-03-29 00:03:11 UTC
Severity:
wishlist
#302079#5
Date:
2005-03-30 01:05:58 UTC
From:
To:
Hello,
  with the attached patch, start-stop-daemon will look for
a ``limit'' file before loading a daemon, and set the ulimits
for the daemon accordingly. Just an example. Let's say
we have an init script that launches:

   start-stop-daemon --start --exec /usr/sbin/adaemon -- $ARGS

With the attacched patch, start-stop-daemon will then look for a
file called ``/etc/limits/adaemon'' containing something like:

  # Ok, limit core file sizes
core    soft    2048
core    hard    4096
nofile  soft    100
nofile  hard    200
#nproc   soft    50
#nproc   hard    150
cpu     soft    12
cpu     hard    15
data    soft    120000
data    hard    135000
#fsize   soft    14000
#fsize   hard    15000
rss     soft    10200
rss     hard    14500
stack   soft    120000
stack   hard    130000
memlock soft    15000
memlock hard    17000
as      soft    10000000
as      hard    10000000

  The patch should work quite fine. It respects the quiet and
verbose flags, prints a warning in case of wrong lines or badly
formatted file, supports comments, and goes on as far as possible
in case of errors, ignores the absence of the file and should
generally behave correctly in most conditions. I believe applying
this patch would maintain complete backward compatibility, and only
add useful functions. It adds ``transparent'' support for setting
ulimits on any daemon launched by start-stop-daemon.

  The patch should include support for *BSD systems, even if I
haven't had the chance to test it on such systems.
I'm a DD too, and if there is any chance for it to be included
in the ``upstream'' dpkg, I'd be willing to work on writing
documentation, verify it works under kfreebsd and hurd, update
it for your current development tree and generally verify its
correct behavior.

  I believe it to be very important to have a system wide
method to set per-daemon limits, considering the always
existing problems about resource exaustion and oom.

  I don't like the resulting code pretty much, but should
be quite reliable (I put lot of attention on it), backward
compatible and generally compliant with the ``coding style''
used for start-stop-daemon.

  I would also love to have a start-stop-daemon with reload
and some lightweight monitoring features, either in the main
dpkg package, or as an enhanced-start-stop-daemon package.

Cheers,
Carlo

#302079#12
Date:
2008-07-04 16:11:12 UTC
From:
To:
Hi Carlo,

sorry for the delay in getting back to you but dpkg has many open bugs...

I like the idea of this patch and it makes much sense to me. So if you're
still interested in updating it and getting it merged, feel free to jump
in! :)

dpkg is now managed with git, see
http://wiki.debian.org/Teams/Dpkg/GitUsage

Why didn't you try to understand what you didn't like and fix it? :)

One thing at a time... :) and it might be that start-stop-daemon will
be mainly useless if we switch to upstart one day (which has such
respawn/restart features).

Anyway, here are a few comments but take them with a grain of salt as I'm
not very used to the dpkg codebase yet.

Do we really need to check for those headers? AFAIK, they are mandated by
POSIX.

If you keep the header checks, just protect each include by its HAVE_*
variable. And use directly the checks HAVE_(S|G)ETRLIMIT in the code
instead of using the intermediary SSD_SETRLIMIT.

See below my remark about the parsing. This should be a simple
function to convert limit name ("as") into the corresponding limit
identifier (RLIMIT_AS) and nothing more complicated.

[ Skip the rest of this function ]

You should use "startas" here. It what's is really used
and should be always set with --start. If it's not set, you
shouldn't do anything anyway.

No need for a fallback with startas, direct stop if not set.


Wouldn't a fixed-size static buffer be simpler (with snprintf)?
FILENAME_MAX / PATH_MAX?

It's not like we'll ever reach any limit with length of daemon names.

Shouldn't we raise an error with fatal instead of silently continuing if
there's another problem (say a permission problem) ?

(I'm not sure of what's best)

Much of that parsing seems fragile/ugly to me. May I suggest to use
fgets() to read a line and then sscanf() to analyze it (with strcmp() to
verify if each field correspond to a valid value)?

And if the first field start with "#" then skip the line.
Same goes if sscanf has not been able to extract the 3 fields.

[ Skipping the rest of the function ]

The parsing should happen first and then the change of limits. Both
shouldn't be too tightly mixed as they are now.

If the hard/soft limits are not set together, you might have troubles
due to incompatibilites between the current values and the new values...
in particular if the soft/hard limit setting happens with two calls to
setrlimit, no?

So maybe it's best to parse the whole file before doing the setrlimit
calls.

Cheers,