Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Incredibly high RLIMIT_NOFILE results in minutes of initial connection delay #600

Open
Earlopain opened this issue Dec 7, 2023 · 6 comments · May be fixed by #601 or #626
Open

Incredibly high RLIMIT_NOFILE results in minutes of initial connection delay #600

Earlopain opened this issue Dec 7, 2023 · 6 comments · May be fixed by #601 or #626
Labels
bug

Comments

@Earlopain
Copy link

Describe the bug
On a system with and incredibly high RLIMIT_NOFILE, establishing a connection can take minutes.

To Reproduce

  1. Start x11vnc
  2. Increase RLIMIT_NOFILE with something like prlimit -n1073741816 -p $(pidof x11vnc)
  3. Connect to x11vnc with a vnc client
  4. Observe that the connection takes incredibly long to establish.

Expected Behavior
The connection speed should be independent from this limit but the following code iterates through each fd instead:

libvncserver/src/libvncserver/sockets.c

Lines 508 to 527 in 784cccb

#if defined LIBVNCSERVER_HAVE_SYS_RESOURCE_H && defined LIBVNCSERVER_HAVE_FCNTL_H
if(getrlimit(RLIMIT_NOFILE, &rlim) < 0)
maxfds = 100; /* use a sane default if getting the limit fails */
else
maxfds = rlim.rlim_cur;
/* get the number of currently open fds as per https://stackoverflow.com/a/7976880/361413 */
curfds = 0;
for(i = 0; i < maxfds; ++i)
if(fcntl(i, F_GETFD) != -1)
++curfds;
if(curfds > maxfds * rfbScreen->fdQuota) {
rfbErr("rfbProcessNewconnection: open fd count of %lu exceeds quota %.1f of limit %lu, denying connection\n", curfds, rfbScreen->fdQuota, maxfds);
sock = accept(chosen_listen_sock, NULL, NULL);
rfbCloseSocket(sock);
return FALSE;
}
#endif

Through a current bug with docker/containerd systemd integration, the docker context inherits this incredibly high limit. This will eventually be fixed, but I still wanted to open an issue about this here since the limit may be set high intentionally. Here's some prior art about the same issue in fakeroot. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=920913
@Earlopain Earlopain added the bug label Dec 7, 2023
@bk138
Copy link
Member

bk138 commented Dec 7, 2023

Thanks for reporting! Do you have a rough idea regarding a fix?

@Earlopain
Copy link
Author

Earlopain commented Dec 7, 2023 •
edited
Loading

This really isn't my forte, so unfortunatly no. The debian issue I linked and similar ones as well didn't do anything itself I believe but relied on this value not being set so high that it causes issues. That is what eventually will happen with the docker issue I mentioned as well.

It's entirely fair for you to just close this and leave it as is if the code is needed like that. Just having this issue here searchable would have saved me quite a few frustrating hours so perhaps some future soul can benefit from it being documented here.

@RokerHRO
Copy link

RokerHRO commented Dec 7, 2023

I'd suggest: If the RLIMIT_NOFILE is set to a pathologically high value – which is a sign that something goes wrong in the system – it should be lowered to a sensible value, e.g. 10k or 100k or the like and the program should call setrlimit() with that sane value immediately.

@bk138
Copy link
Member

bk138 commented Dec 7, 2023

Hah, I was about to write I didn't know about the original author's intentions, but the commit is in fact by a previous me 37a5810 🤔

@Earlopain
Copy link
Author

Earlopain commented Dec 7, 2023 •
edited
Loading

Hm, as far as I understand this is supposed to guard against fd exhaustion.

So, if say 10k (or some other arbitrarily high number) of unused fds were found, the chance of that seems pretty low. Would it be an alternative to bail out early if sufficient free slots were found and skip checking fds further down?

@Earlopain Earlopain mentioned this issue Dec 8, 2023
Closed
Earlopain added a commit to Earlopain/libvncserver that referenced this issue Dec 8, 2023
@Earlopain
Stop counting fds after enough free ones are encountered
efb7d01 
On systems with incredibly high `RLIMIT_NOFILE` (either intentionally or
by accident) this can take quite a large amount of time.
Closes LibVNC#600
Earlopain added a commit to Earlopain/libvncserver that referenced this issue Dec 8, 2023
@Earlopain
Stop counting fds after enough free ones are encountered
95023d2 
On systems with incredibly high `RLIMIT_NOFILE` (either intentionally or
by accident) this can take quite a large amount of time.

Closes LibVNC#600
Earlopain added a commit to Earlopain/libvncserver that referenced this issue Dec 8, 2023
@Earlopain
Stop counting fds after enough free ones are encountered
a65ad8b 
On systems with incredibly high `RLIMIT_NOFILE` (either intentionally or
by accident) this can take quite a large amount of time.

Closes LibVNC#600
@Earlopain Earlopain linked a pull request Dec 8, 2023 that will close this issue
Open
@ethan-vanderheijden ethan-vanderheijden linked a pull request Jul 24, 2024 that will close this issue
Open
@ethan-vanderheijden
Copy link

I ran into this bug too! It turns out, if you run a process inside a Docker container, the file descriptor limit is in the millions or billions. That means millions or billions of calls to fcntl... which takes a long, long time.

This is very similar to a bug in Red Hat's Package Manager (rpm), and their solution was to avoid calling fcntl and instead count the file inside /proc/self/fd/ (see rpm-software-management/rpm#444). This solution only works on Linux, but it is far better than checking every possible file descriptor.

I proposed my own fix in #626

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
4 participants
@bk138 @Earlopain @RokerHRO @ethan-vanderheijden