This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Wrong computation of max_fd on OpenBSD
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ced, davin, gregory.p.smith, python-dev, skrah, worr
Priority: normal Keywords: patch

Created on 2015-04-02 15:13 by ced, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
fd_dir.patch ced, 2015-04-02 15:13 review
max_fd.patch ced, 2015-04-03 18:24 review
max_fd.patch ced, 2015-04-04 14:25 review
max_fd.patch worr, 2015-04-24 20:48 review
Messages (17)
msg239920 - (view) Author: Cédric Krier (ced) * Date: 2015-04-02 15:13
The test_subprocess fails since issue21618 on OpenBSD because the FD_DIR is wrong (/dev/fd instead of /proc/self/fd).
msg240002 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-04-03 15:31
There's a comment in _posixsubprocess:

"NetBSD and OpenBSD have a /proc fs available (though not necessarily
mounted) and do not have fdescfs for /dev/fd."


Is this still valid?
msg240005 - (view) Author: Cédric Krier (ced) * Date: 2015-04-03 15:53
At least on OpenBSD procfs have been removed: http://www.openbsd.org/faq/current.html#20140908
msg240013 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2015-04-03 17:33
I'm not going to bother setting up a VM with an esoteric OS in it myself, if someone knows the past and current state of various OpenBSD versions and what to do there, feel free to commit OpenBSD conditional compilation bits as you see fit to make this do the right thing. :)
msg240017 - (view) Author: Cédric Krier (ced) * Date: 2015-04-03 17:50
Indeed, I think my patch is not right as /dev/fd on OpenBSD is static.
I will continue to investigate.
msg240020 - (view) Author: Cédric Krier (ced) * Date: 2015-04-03 18:11
The problem comes from safe_get_max_fd which return a too low value because of a bug in sysconf on OpenBSD [1]:

The value for _SC_STREAM_MAX is a minimum maximum, and required to be the same as ANSI C's FOPEN_MAX, so the returned value is a ridiculously small and misleading number.

[1] http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/sysconf.3
msg240023 - (view) Author: Cédric Krier (ced) * Date: 2015-04-03 18:24
Here is a patch that uses getrlimit (that's works on OpenBSD) before using sysconf.
msg240068 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-04-04 12:54
Unfortunately I don't have an OpenBSD install either.  From the
sysconf.c source ...

  http://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/src/lib/libc/gen/sysconf.c?rev=1.22&content-type=text/plain


... it seems that sysconf(_SC_OPEN_MAX) also calls getrlimit().
msg240075 - (view) Author: Cédric Krier (ced) * Date: 2015-04-04 14:24
But sysconf(_SC_OPEN_MAX) uses rlim_cur which is too low instead of rlim_max. My proposal is indeed describe in msg219477, it is not prefect but at least better than the current one for OpenBSD.
msg240076 - (view) Author: Cédric Krier (ced) * Date: 2015-04-04 14:25
Correctly cast to long instead of int.
msg240619 - (view) Author: William Orr (worr) * Date: 2015-04-13 15:10
Tested on OpenBSD 5.6/amd64. lgtm.
msg240772 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2015-04-13 21:46
getrlimit() is not an async-signal-safe function according to http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html so you cannot call it from safe_get_max_fd().

having the getrlimit call done prior to the fork and using the value returned by that iff neither of the other two methods (fcntl and sysconf) are available or produced results seems like the best you can do.

also, rlim_t is an unsigned value yet the existing code in this module is using signed values for the file descriptors.  realistically i do not expect an rlim_t for max file descriptors to ever be > MAX_LONG as many file descriptor API calls require signed values for fds in order to use -1 as an error.

But checking the value for overflow and against the RLIM constants mentioned in http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/resource.h.html before casting it via (long) seems pedantically wise.
msg240929 - (view) Author: William Orr (worr) * Date: 2015-04-14 16:12
Given that OpenBSD returns *bad* data via sysconf(3), I'm not sure that there's a good way to validate other than *only* calling getrlimit(3) on OpenBSD.

Is that an acceptable approach?
msg241035 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2015-04-14 21:43
yeah, that's fine.  just surround the call to getrlimit with appropriate openbsd ifdef's and a comment.  it is _probably_ async signal safe given the nature of the function in most implementations even though it isn't on the official posix list (many things are).
msg241394 - (view) Author: William Orr (worr) * Date: 2015-04-18 03:47
I've updated Cédric's patch to only run that portion on OpenBSD.
msg241982 - (view) Author: William Orr (worr) * Date: 2015-04-24 20:48
Updated the patch based on review
msg242047 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-04-26 06:44
New changeset 7df280b311d0 by Gregory P. Smith in branch '3.4':
Fix computation of max_fd on OpenBSD.  Issue #23852.
https://hg.python.org/cpython/rev/7df280b311d0

New changeset 08d0cc23fb00 by Gregory P. Smith in branch 'default':
Fix computation of max_fd on OpenBSD.  Issue #23852.
https://hg.python.org/cpython/rev/08d0cc23fb00
History
Date User Action Args
2022-04-11 14:58:15adminsetgithub: 68040
2015-04-26 09:19:49berker.peksagsetstage: patch review -> resolved
2015-04-26 06:47:00gregory.p.smithsetstatus: open -> closed
resolution: fixed
2015-04-26 06:44:20python-devsetnosy: + python-dev
messages: + msg242047
2015-04-24 20:48:18worrsetfiles: + max_fd.patch

messages: + msg241982
2015-04-24 20:48:00worrsetfiles: - max_fd.patch
2015-04-18 03:47:42worrsetfiles: + max_fd.patch

messages: + msg241394
2015-04-14 21:43:30gregory.p.smithsetmessages: + msg241035
2015-04-14 16:12:38worrsetmessages: + msg240929
2015-04-13 21:46:02gregory.p.smithsetmessages: + msg240772
2015-04-13 15:10:28worrsetnosy: + worr
messages: + msg240619
2015-04-04 14:25:52cedsetfiles: + max_fd.patch

messages: + msg240076
2015-04-04 14:24:43cedsetmessages: + msg240075
2015-04-04 12:54:39skrahsetmessages: + msg240068
2015-04-04 10:02:12cedsetfiles: - max_fd.patch
2015-04-04 10:02:00cedsetfiles: + max_fd.patch
title: Wrong FD_DIR file name on OpenBSD -> Wrong computation of max_fd on OpenBSD
2015-04-03 18:31:15cedsettype: behavior
2015-04-03 18:24:09cedsetfiles: + max_fd.patch

messages: + msg240023
2015-04-03 18:11:17cedsettype: behavior -> (no value)
messages: + msg240020
2015-04-03 17:50:56cedsetmessages: + msg240017
2015-04-03 17:33:18gregory.p.smithsetversions: + Python 3.4, Python 3.5
messages: + msg240013

components: + Extension Modules
type: behavior
stage: patch review
2015-04-03 15:53:15cedsetmessages: + msg240005
2015-04-03 15:32:10skrahsetnosy: + gregory.p.smith
2015-04-03 15:31:15skrahsetnosy: + skrah
messages: + msg240002
2015-04-02 16:09:12davinsetnosy: + davin
2015-04-02 15:13:52cedcreate