classification
Title: EINTR handling in fcntl
Type: behavior Stage:
Components: Extension Modules, IO Versions: Python 3.6, Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Jack Zhou, neologix, python-dev, serhiy.storchaka, twouters, vstinner
Priority: normal Keywords: patch

Created on 2016-04-08 19:50 by Jack Zhou, last changed 2016-04-17 15:16 by vstinner.

Files
File name Uploaded Description Edit
fcntl_eintr.patch vstinner, 2016-04-09 09:54 review
fcntl_eintr-2.patch vstinner, 2016-04-15 16:28 review
fcntl_eintr-3.patch vstinner, 2016-04-17 15:15 review
Messages (13)
msg263042 - (view) Author: Jack Zhou (Jack Zhou) Date: 2016-04-08 19:50
According to PEP 475, standard library modules should handle EINTR, but this appears to not be the case for the fcntl module.

Test script:


import fcntl
import signal
import os


def handle_alarm(signum, frame):
    print("Received alarm in process {}!".format(os.getpid()))


child = os.fork()
if child:
    signal.signal(signal.SIGALRM, handle_alarm)
    signal.alarm(1)
    with open("foo", "w") as f:
        print("Locking in process {}...".format(os.getpid()))
        fcntl.flock(f, fcntl.LOCK_EX)
        print("Locked in process {}.".format(os.getpid()))
        os.waitpid(child, 0)
else:
    signal.signal(signal.SIGALRM, handle_alarm)
    signal.alarm(1)
    with open("foo", "w") as f:
        print("Locking in process {}...".format(os.getpid()))
        fcntl.flock(f, fcntl.LOCK_EX)
        print("Locked in process {}.".format(os.getpid()))
msg263075 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-04-09 09:52
New changeset 60ac28b612af by Victor Stinner in branch '3.5':
Update fcntl doc: replace IOError with OSError
https://hg.python.org/cpython/rev/60ac28b612af
msg263076 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-09 09:54
Oh, we forgot the fcntl module in the PEP 475.

Here is a fix for Python 3.5 (and 3.6).
msg263078 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-04-09 10:26
It looks to me that interrupting fcntl can be used for implementing blocking lock with a timeout. If automatically retry fcntl() on EINTR, this will break such code.
msg263086 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-09 12:33
Serhiy Storchaka added the comment:
> It looks to me that interrupting fcntl can be used for implementing
blocking lock with a timeout. If automatically retry fcntl() on EINTR, this
will break such code.

If the signal handler doesn't raise an exception, the code is not reliable.
It's a common complain about the PEP 475.

If the signal handler raises an exception, it's work on Python 2.7 and on
Python 3.5 with my patch.
msg263134 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-04-10 12:01
The signal handler doesn't raise an exception in the example on StackOverflow. [1]

I think it would be nice to publish somewhere (on ActoveState receipts, on StackOverflow) well-searchable correct example.

[1] http://stackoverflow.com/questions/5255220/fcntl-flock-how-to-implement-a-timeout
msg263258 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-12 15:54
> I think it would be nice to publish somewhere (on ActoveState receipts, on StackOverflow) well-searchable correct example.

Yeah, it's always possible to enhance the doc. I'm not an user of ActiveState or StackOverflow websites. Don't hesitate to submit a comment to explain the PEP 475 ;-)

Is it ok to fix Python 3.5? Or is it safer to only fix the issue in Python 3.6?
msg263271 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-04-12 19:04
I just don't know whether the patch will break existing code that relies on current behavior. If EINTR handling in other functions was not backported to older versions, I think this is good argument against fixing this issue in 3.5.

Could you write tests with signal handlers raising and not raising an exception?
msg263277 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-12 20:44
Ok, I will update the patch to include an unit test and only apply it
to Python 3.5.
msg263391 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-04-14 09:31
New changeset 9ffe055f2b0e by Serhiy Storchaka in branch '3.5':
Issue #26716: Regenerate Argument Clinic code.
https://hg.python.org/cpython/rev/9ffe055f2b0e

New changeset 19dec08e54a8 by Serhiy Storchaka in branch 'default':
Issues #26716, #26057: Regenerate Argument Clinic code.
https://hg.python.org/cpython/rev/19dec08e54a8
msg263507 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-15 16:28
Patch version 2:

* add a comment to explain the C loop retrying on EINTR error
* add an unit test: I checked that the test fails with InterruptedError without the fix
* document the change in fcntl documentation
msg263512 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-04-15 18:01
Left nitpicks about tests.
msg263613 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-17 15:16
I updated the test, does it look better now?
History
Date User Action Args
2016-04-17 15:16:04vstinnersetmessages: + msg263613
2016-04-17 15:15:24vstinnersetfiles: + fcntl_eintr-3.patch
2016-04-15 18:01:03serhiy.storchakasetmessages: + msg263512
2016-04-15 16:28:55vstinnersetfiles: + fcntl_eintr-2.patch

messages: + msg263507
2016-04-14 09:31:23python-devsetmessages: + msg263391
2016-04-12 20:44:39vstinnersetmessages: + msg263277
2016-04-12 19:04:48serhiy.storchakasetmessages: + msg263271
2016-04-12 15:54:35vstinnersetmessages: + msg263258
2016-04-10 12:01:33serhiy.storchakasetmessages: + msg263134
2016-04-09 12:33:42vstinnersetmessages: + msg263086
2016-04-09 10:26:33serhiy.storchakasetmessages: + msg263078
2016-04-09 09:54:25vstinnersetfiles: + fcntl_eintr.patch

nosy: + serhiy.storchaka
messages: + msg263076

keywords: + patch
2016-04-09 09:52:22python-devsetnosy: + python-dev
messages: + msg263075
2016-04-08 20:14:30SilentGhostsetnosy: + twouters, vstinner, neologix

components: + Extension Modules
versions: + Python 3.6
2016-04-08 19:50:33Jack Zhoucreate