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.

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

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

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

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

Earlopain opened this issue Dec 7, 2023 · 5 comments · May be fixed by #601
Labels

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:

#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

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

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 added a commit to Earlopain/libvncserver that referenced this issue Dec 8, 2023
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
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
On systems with incredibly high `RLIMIT_NOFILE` (either intentionally or
by accident) this can take quite a large amount of time.

Closes LibVNC#600
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants