Created on 2014-08-23 13:20 by igor.pashev, last changed 2014-09-04 07:30 by haypo. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| dyson-set_inheritable.patch | igor.pashev, 2014-08-23 13:20 | Patch making use of FD_CLOEXEC in set_inheritable() | ||
| ioctl_works.patch | haypo, 2014-08-25 01:00 | review | ||
| Messages (12) | |||
|---|---|---|---|
| msg225745 - (view) | Author: Igor Pashev (igor.pashev) * | Date: 2014-08-23 13:20 | |
I've found on illumos-based OS that under some conditions python is unable to open any files because set_inheritable() fails. This happens even when building python when it cannot find (open) _sysconfigdata.py. The problem is that set_inheritable() first checks for FIOCLEX/FIONCLEX ioctls and tries to use them if such macros are defined. These macros can be defined at illumos, but kernel does not support them (inappropriate ioctl). Since other pieces of python already use FD_CLOEXEC unconditionally, including get_inheritable() I patched set_inheritable() to use FD_CLOEXEC. See attached patch. |
|||
| msg225765 - (view) | Author: STINNER Victor (haypo) *  |
Date: 2014-08-23 21:27 | |
The patch doesn't look correct. Ioctl() reduces the overhead in term of syscalls (1 vs 2). What is the errno value on failure? We should remember that ioctl() doesn't work and fallback to fcntl(). It would be more portable. Similar check is already done for O_TMPFILE and O_CLOEXEC for example. See the PEP 446. |
|||
| msg225799 - (view) | Author: Igor Pashev (igor.pashev) * | Date: 2014-08-24 07:39 | |
errno is 25 (#define ENOTTY 25 /* Inappropriate ioctl for device */) It does not make sense to me to call unworkable ioctl() each time before other methods :-) I would consider adding a configure check for working ioctl() (but it won't work for cross compilation). |
|||
| msg225813 - (view) | Author: STINNER Victor (haypo) * |
Date: 2014-08-24 12:41 | |
I propose to only call ioctl() once, and then remember (in a static variable) that it doesn't work and then always call fcntl(). I can work on a patch. |
|||
| msg225855 - (view) | Author: STINNER Victor (haypo) * |
Date: 2014-08-25 01:00 | |
@Igor: Here is a patch. Can you please apply it and run "./python -m test -v test_os" on your OS? I tried it manually on Linux by forcing the errno to ENOTTY. |
|||
| msg226265 - (view) | Author: STINNER Victor (haypo) * |
Date: 2014-09-02 07:53 | |
Ping Igor. |
|||
| msg226267 - (view) | Author: Igor Pashev (igor.pashev) * | Date: 2014-09-02 08:38 | |
Yes, Victor. Your patch works. All os tests are passed :-) |
|||
| msg226268 - (view) | Author: Roundup Robot (python-dev) | Date: 2014-09-02 09:50 | |
New changeset 27cef7476f2b by Victor Stinner in branch '3.4': Closes #22258: Fix the the internal function set_inheritable() on Illumos. http://hg.python.org/cpython/rev/27cef7476f2b New changeset 4a51c45f405b by Victor Stinner in branch 'default': (Merge 3.4) Closes #22258: Fix the the internal function set_inheritable() on http://hg.python.org/cpython/rev/4a51c45f405b |
|||
| msg226269 - (view) | Author: STINNER Victor (haypo) * |
Date: 2014-09-02 09:50 | |
> Yes, Victor. Your patch works. All os tests are passed :-) Thanks for the tests. I pushed my fix to Python 3.4 & 3.5. |
|||
| msg226337 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-09-04 06:27 | |
> +- Issue #22258: Fix the the internal function set_inheritable() on Illumos. "the the" |
|||
| msg226339 - (view) | Author: Roundup Robot (python-dev) | Date: 2014-09-04 07:29 | |
New changeset 9d5386a22e68 by Victor Stinner in branch 'default': Issue #22258: Fix typo in Misc/NEWS http://hg.python.org/cpython/rev/9d5386a22e68 |
|||
| msg226340 - (view) | Author: STINNER Victor (haypo) * |
Date: 2014-09-04 07:30 | |
> "the the" Oh, thanks for the report. It's now fixed. It would be better to have the Misc/NEWS entry in the patch directly :-( |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2014-09-04 07:30:27 | haypo | set | messages: + msg226340 |
| 2014-09-04 07:29:48 | python-dev | set | messages: + msg226339 |
| 2014-09-04 06:27:13 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages: + msg226337 |
| 2014-09-02 09:50:57 | haypo | set | stage: resolved -> messages: + msg226269 versions: + Python 3.5 |
| 2014-09-02 09:50:08 | python-dev | set | status: open -> closed nosy: + python-dev messages: + msg226268 resolution: fixed stage: resolved |
| 2014-09-02 08:38:33 | igor.pashev | set | messages: + msg226267 |
| 2014-09-02 07:53:07 | haypo | set | messages: + msg226265 |
| 2014-08-25 01:00:12 | haypo | set | files:
+ ioctl_works.patch messages: + msg225855 |
| 2014-08-25 00:51:05 | haypo | set | title: Use FD_CLOEXEC in Python/fileutils.c -> set_inheritable(): ioctl(FIOCLEX) is available but fails with ENOTTY on Illumos |
| 2014-08-24 12:41:12 | haypo | set | messages: + msg225813 |
| 2014-08-24 07:39:10 | igor.pashev | set | messages: + msg225799 |
| 2014-08-23 21:27:29 | haypo | set | messages: + msg225765 |
| 2014-08-23 13:47:44 | pitrou | set | nosy:
+ haypo |
| 2014-08-23 13:20:24 | igor.pashev | create | |