Bug 5554: Cap raw RLIMIT_NOFILE in max_filedescriptors default by mxschmitt · Pull Request #2483 · squid-cache/squid

mxschmitt · 2026-08-26T00:50:50Z

setMaxFD() updates Squid_MaxFD. Squid_MaxFD is then used to allocate
various FD-indexed tables, including Comm's fd_table. When
Config.max_filedescriptors is not set, Squid has to guess the maximum
number of descriptors this instance can handle. Prior to these changes,
that guess was based on OS-provided ulimits (RLIMIT_NOFILE).

Using raw RLIMIT_NOFILE values to set Squid_MaxFD results in huge
fd_table allocations that kill or incapacitate Squid when OS uses very
large limits (e.g., Kubernets soft limit of 1073741816=2^30-8
effectively means "unlimited" and results in ~432 MB fd_table). We now
cap such raw values at 100K which yields ~42 MB fd_table.

Several other old problems were discovered and marked during this work.
Those out-of-scope problems deserve dedicated fixes.

N.B. setMaxFD() calls setrlimit(2) in some cases, but the primary/final
setrlimit(2) call is in setSystemLimits(). That primary call sets the
soft limit to Squid_MaxFD.

kinkie · 2026-08-26T07:26:09Z

    return result;
 }
 #endif // _SQUID_WINDOWS_ || _SQUID_MINGW_
-


Please restore the empty line at the end of the file.
Apart from this, looks good to me!

Please restore the empty line at the end of the file.

I will fix this while working on the adjustments mentioned in another change request.

Max fixed this in branch commit e298f73.

rousskov

Bug report: Squid should not blindly trust an absurd inherited soft limit and allocate unbounded descriptor tables before startup completes. It should cap an inherited default to a safe compiled/runtime limit, or otherwise reject it with a clear diagnostic.

Thank you for this bug report and analysis, Max! I agree with the above conclusion and support this PR direction. Some PR changes are required, but I will implement them myself in hope to reduce the number of review iterations and cumulative time spent on this PR. Please stand by -- the ball is in my court.

Since this is your first Squid contribution, I will also add your entry to the CONTRIBUTORS file (unless you stop me).

rousskov · 2026-08-26T15:11:56Z

    return result;
 }
 #endif // _SQUID_WINDOWS_ || _SQUID_MINGW_
-


Please restore the empty line at the end of the file.

I will fix this while working on the adjustments mentioned in another change request.

mxschmitt · 2026-08-26T15:21:01Z

Thanks @rousskov for taking over - highly appreciate your time you put into this project - really big fan of squid and using it in one of my side projects.

rousskov · 2026-08-28T18:03:37Z

I am still working on this PR adjustments, but I want to double check something:

Original PR description: The later SQUID_MAXFD_LIMIT clamp occurs after these allocations.

@mxschmitt, AFAICT, the above assertion is false. In other words, SQUID_MAXFD_LIMIT clamping is done (clamping the default Squid_MaxFD value), before setMaxFD() is called and before "these allocations" happen. Could you please double check? If I am right, then there is another Squid bug here, but fixing that bug would be outside this PR scope.

Even if that PR description claim is wrong, the overall direction of this PR remains correct; earlier clamping does not invalidate what this PR is doing...

mxschmitt · 2026-08-28T18:41:24Z

Thanks for catching that — you are correct about the ordering. I corrected the PR description.

The relevant sequence is:

SquidMain applies SQUID_MAXFD_LIMIT
  -> setMaxFD() reads inherited RLIMIT_NOFILE
  -> setMaxFD() overwrites Squid_MaxFD
  -> fde::Init()/callback tables allocate

The patch now caps the inherited default against the already-effective Squid_MaxFD, preserving any lower SQUID_MAXFD_LIMIT while preventing the unbounded inherited value from reaching those allocations. Explicit max_filedescriptors remains unchanged.

mxschmitt · 2026-08-28T18:44:11Z

Restored the empty line at EOF in commit e298f73 and pushed it to the PR branch.

rousskov · 2026-08-28T21:46:16Z

Restored the empty line at EOF in commit e298f73 and pushed it to the PR branch.

Please let me finish my changes before pushing more changes. We are doing the same work twice.

This reverts commit 9c4cba0. The reverted commit effectively addressed the following concern: setMaxFd() should not increase Squid_MaxFD beyond SQUID_MAXFD_LIMIT (by default). That concern should not be addressed here because official code does not follow that logic, and changing that logic is outside this branch scope: * Official code: Raw getrlimit() overwrites SQUID_MAXFD_LIMIT. * Branch code: Capped getrlimit() overwrites SQUID_MAXFD_LIMIT. In other words, we are only concerned with capping getrlimit() results here so that we do not use "absurd" ones. Overwriting SQUID_MAXFD_LIMIT is probably an old bug in the official setMaxFD() going back to 2007 squid2 commit 77000de code that was ported to squid3 in 2010 commit f3f0f56. Should we enlarge this branch scope to include that old bug fix? I believe that the correct answer is "no". It is best to fix that bug in a dedicated PR because a proper fix requires more changes: SQUID_MAXFD_LIMIT caps Squid_MaxFD by FD_SETSIZE to protect ModSelect and ModPoll, but * official ModPoll code does not use Squid_MaxFD (it uses SQUID_MAXFD), rendering that protection attempt/code misleading/unused for ModPoll; * ModPoll does not actually need that protection since 2007 commit 889579f that removed fd_set/DELAY_POOLS from ModPoll! We should get rid of SQUID_MAXFD in ModPoll, but that would significantly increase this branch footprint and make the primary branch changes quite a bit more difficult to grok. We should also get rid of SQUID_MAXFD in ModEpoll, ipc.cc, and CollapsedForwarding.cc! The upcoming branch changes (that this reversal makes easier to merge) will mark this old official bug with an XXX. Notes for future fixes ---------------------- When SQUID_MAXFD_LIMIT was introduced in 2006 commit b926988, setMaxFD() never increased Squid_MaxFD. It could only decrease Squid_MaxFD (when Squid_MaxFD exceeded the hard limit). Thus, that original code worked as intended, offering ModPoll and ModSelect protections. 2010 commit f3f0f56 flipped that logic to "obey whatever ulimit settings as before". The "as before" part of that claim was incorrect. This commit backported Squid v2 changes that contained the same bug (since squid2 commit 77000de).

Using SQUID_MAXFD as Squid_MaxFD in environments with huge soft RLIMIT_NOFILE limits is not a good idea for several reasons: * If ./configure can only open a few (e.g., 1024) files due to tight restrictions in (or insufficient SQUID_CHECK_MAXFD() support for) the build environment, then this branch would make Squid_MaxFD much smaller than it currently is in Squid execution environments that allow for a lot more files (e.g., 32K). For busy deployments using ModDevPoll (that uses Squid_MaxFD rather than SQUID_MAXFD), that decrease would result in a significant regression. Those admins should use max_filedescriptors directive instead of relying on the defaults, but we do not want to break their (unrelated) setups when adding support for huge ulimits! * It introduces or increases an implicit dependency on ./configure to determine a _reasonable_ number of file descriptors rather than a true maximum. Currently, ./configure caps SQUID_MAXFD at 32K while many (most?) modern production environments can open 100K or even 1M files. If ./configure code is adjusted to detect a true maximum, then using SQUID_MAXFD would again result in excessively large FD tables, effectively undoing this branch improvements. * It might tempt admins into manipulating ./configure environment or using `--with-filedescriptors` to control Squid_MaxFD, which we do not really support well (e.g., smaller soft ulimits would result in Squid_MaxFD being smaller than ./configure-set SQUID_MAXFD). Folks should use max_filedescriptors directive instead. * Long-term, we should probably remove SQUID_MAXFD and its guessing ./configure code that leads to many inconsistencies and complications: ModEpoll: SQUID_MAXFD ModPoll: SQUID_MAXFD ModDevPoll: Squid_MaxFD ModSelect: Squid_MaxFD ModKqueue: getdtablesize() ipc.cc "close all": SQUID_MAXFD ipc_win32.cc: Squid_MaxFD WIN32_Subsystem_Init(): Squid_MaxFD TcpAcceptor.cc: listening queue Squid_MaxFD/4. peerCanOpenMore(): Squid_MaxFD Comm: numerous assert(fd < Squid_MaxFD) Comm fde::Table: Squid_MaxFD Comm TheHalfClosed: Squid_MaxFD Comm MakeCallbackTable(): Squid_MaxFD Also added and adjusted branch-added comments to avoid implying that large ulimits are always "unexpected", avoid introducing "inherited ulimit" terminology, and avoid adding misplaced comments regarding setMaxFD()/fed::Init() calls dependencies.

Describe how various errors are handled.

This fixes a branch-added bug.

Squid's guessing algorithm currently ignores the value guessed or set at `./configure` time (e.g., see `--with-filedescriptors`). Add the corresponding XXX.

For example, it does not include CollapsedForwarding::HandleNewData().

This will also help making these `rl` variables constant (eventually).

rousskov

@mxschmitt, I am done with my changes. The primary changes are in branch commits aefc8b8 and ce8c540; they are accompanied by detailed commit messages explaining my rationale. Reviewing individual git commits is not necessary and can get tricky/messy, but please do take a look at the final diff (including cf.data.pre documentation changes) and the updated PR title/description. If you do not like something, please adjust as needed!

I am approving the current PR state, but I do not plan on clearing this PR for merging until you have a chance to review and adjust it.

rousskov · 2026-08-31T15:14:38Z

    return result;
 }
 #endif // _SQUID_WINDOWS_ || _SQUID_MINGW_
-


Max fixed this in branch commit e298f73.

mxschmitt · 2026-09-01T14:48:40Z

Looks great! Tested it as well and works as expected. Thank you!

setMaxFD() updates Squid_MaxFD. Squid_MaxFD is then used to allocate various FD-indexed tables, including Comm's `fd_table`. When Config.max_filedescriptors is not set, Squid has to guess the maximum number of descriptors this instance can handle. Prior to these changes, that guess was based on OS-provided ulimits (RLIMIT_NOFILE). Using raw RLIMIT_NOFILE values to set Squid_MaxFD results in huge fd_table allocations that kill or incapacitate Squid when OS uses very large limits (e.g., Kubernets soft limit of 1073741816=2^30-8 effectively means "unlimited" and results in ~432 MB fd_table). We now cap such raw values at 100K which yields ~42 MB fd_table. Several other old problems were discovered and marked during this work. Those out-of-scope problems deserve dedicated fixes. N.B. setMaxFD() calls setrlimit(2) in some cases, but the primary/final setrlimit(2) call is in setSystemLimits(). That primary call sets the soft limit to Squid_MaxFD.

squidadm · 2026-09-01T18:19:30Z

queued for backport to v7

squid-anubis added the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Aug 26, 2026

This comment was marked as resolved.

Sign in to view

mxschmitt force-pushed the fix/inherited-nofile-cap branch from 7f6d150 to 3ad247b Compare August 26, 2026 00:54

This comment was marked as resolved.

Sign in to view

Bug 5554: Cap absurd inherited RLIMIT_NOFILE

c041695

mxschmitt force-pushed the fix/inherited-nofile-cap branch from 3ad247b to c041695 Compare August 26, 2026 00:56

squid-anubis removed the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Aug 26, 2026

mxschmitt mentioned this pull request Aug 26, 2026

squid: cap inherited file descriptor limit mxschmitt/try-playwright#306

Merged

kinkie reviewed Aug 26, 2026

View reviewed changes

kinkie added S-waiting-for-author author action is expected (and usually required) backport-to-v7 maintainer has approved these changes for v7 backporting labels Aug 26, 2026

rousskov requested changes Aug 26, 2026

View reviewed changes

rousskov added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Aug 26, 2026

rousskov self-requested a review August 26, 2026 15:18

Bug 5554: preserve effective descriptor limit

9c4cba0

squid-anubis added the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Aug 28, 2026

This comment was marked as resolved.

Sign in to view

Bug 5554: restore source file trailing blank line

e298f73

rousskov added 5 commits August 31, 2026 10:36

Added in-scope and out-of-scope XXXs

98928f6

Synced and improved max_filedescriptors directive docs

d9b9cef

Describe how various errors are handled.

fixup: Do cap the limit if we fail to set it to max_filedescriptors

b0c5c3f

This fixes a branch-added bug.

rousskov added 4 commits August 31, 2026 10:36

fixup: Added primary author to CONTRIBUTORS

2466543

fixup: Removed incorrect branch-added doc statement

3fc589b

Squid's guessing algorithm currently ignores the value guessed or set at `./configure` time (e.g., see `--with-filedescriptors`). Add the corresponding XXX.

fixup: SQUID_MAXFD users list was not meant to be exhaustive

ae0685c

For example, it does not include CollapsedForwarding::HandleNewData().

fixup: Avoid setting rl.rlim_cur just to set Squid_MaxFD.

a7abc86

This will also help making these `rl` variables constant (eventually).

squid-anubis removed the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Aug 31, 2026

rousskov changed the title ~~Bug 5554: Cap absurd inherited RLIMIT_NOFILE~~ Bug 5554: Cap raw RLIMIT_NOFILE in max_filedescriptors default Aug 31, 2026

fixup: AAA

0417ba0

rousskov approved these changes Aug 31, 2026

View reviewed changes

rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Aug 31, 2026

kinkie approved these changes Sep 1, 2026

View reviewed changes

rousskov added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-author author action is expected (and usually required) labels Sep 1, 2026

squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Sep 1, 2026

squid-anubis closed this Sep 1, 2026

squidadm removed the backport-to-v7 maintainer has approved these changes for v7 backporting label Sep 1, 2026

Uh oh!

Conversation

mxschmitt commented Aug 26, 2026 • edited by rousskov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

kinkie Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

rousskov Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

rousskov Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

rousskov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rousskov Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

mxschmitt commented Aug 26, 2026

Uh oh!

rousskov commented Aug 28, 2026

Uh oh!

This comment was marked as resolved.

mxschmitt commented Aug 28, 2026

Uh oh!

mxschmitt commented Aug 28, 2026

Uh oh!

rousskov commented Aug 28, 2026

Uh oh!

rousskov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rousskov Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mxschmitt commented Sep 1, 2026

Uh oh!

squidadm commented Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mxschmitt commented Aug 26, 2026 •

edited by rousskov

Loading