Feh / nocache

minimize caching effects
BSD 2-Clause "Simplified" License
554 stars 53 forks source link

nocache.c:148: init_mutexes: Assertion `fds_lock != NULL' failed. #38

Closed onlyjob closed 5 years ago

onlyjob commented 5 years ago

As reported in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=918464#50 nocache may fail as follows:

$ timeout 11 ./nocache apt show coreutils 1>>/dev/null
apt: nocache.c:148: init_mutexes: Assertion `fds_lock != NULL' failed.
Aborted
Error 134

Here is what Sven Joachim wrote about it:

The good news is that I seem to have found the explanation for the failed assertion. In line 147 of nocache.c we have

fds_lock = malloc(max_fds * sizeof(*fds_lock));

and malloc obviously returned NULL. With a debug printf statement I found out that max_fds == 1073741816, with sizeof(*fds_lock) == 40 it is not too surprising that malloc failed.

Why is max_fds so high? In the systemd changelog I found out the following:

,----
| systemd (240-2) unstable; urgency=medium
| 
|   * Don't bump fs.nr_open in PID 1.
|     In v240, systemd bumped fs.nr_open in PID 1 to the highest possible
|     value. Processes that are spawned directly by systemd, will have
|     RLIMIT_NOFILE be set to 512K (hard).
|     pam_limits in Debian defaults to "set_all", i.e. for limits which are
|     not explicitly configured in /etc/security/limits.conf, the value from
|     PID 1 is taken, which means for login sessions, RLIMIT_NOFILE is set to
|     the highest possible value instead of 512K. Not every software is able
|     to deal with such an RLIMIT_NOFILE properly.
|     While this is arguably a questionable default in Debian's pam_limit,
|     work around this problem by not bumping fs.nr_open in PID 1.
|     (Closes: #917167)
| 
|  -- Michael Biebl <biebl@debian.org>  Thu, 27 Dec 2018 14:03:57 +0100
`----

And this sid system has an uptime of 13 days, so was booted with systemd 240-1 which explains the high RLIMIT_NOFILE. On a freshly booted laptop, I get max_fds == 1048576 instead, and obviously malloc'ing 40 Megabytes rather than 40 Gigabytes of RAM is easily possible.

Feh commented 5 years ago

I see three options:

  1. Change the data structure of locks to some map-like container or a search tree, using the fd as a key. This scales well for a low number of FDs, but has the same (or worse) problems when you actually want to use a billion FDs. The drawback is that you need to allocate heap memory during every system call interception.
  2. Introduce an artificial cap on max_fds – make nocache work on the first N FDs, where we could pick N to be a million or 100k. That makes the fix trivial. We can implement a flag for this if people care.
  3. Do nothing because this is a pathological case.

Given that it caused build failures for some people, 3) is not a real option I think. Option 2) is easy to implement correctly.

Thoughts?

Feh commented 5 years ago

I’ll take silence as assent and merge these two commits.

shadjiiski commented 4 years ago

@Feh, sorry for digging in old issues. Looking in the getrlimit documentation, I believe the off-by-one issue mentioned in your commit message was introduced (instead of fixed). Left a comment for you to review in the code: https://github.com/Feh/nocache/commit/ce5c18701b1499195d0de7380037efee27a17722#r39007823