classification
Title: errno is read too late
Type: behavior Stage: resolved
Components: None Versions: Python 3.2, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, giampaolo.rodola, hfuru, pitrou, python-dev, terry.reedy
Priority: normal Keywords: patch

Created on 2010-11-08 08:48 by hfuru, last changed 2011-12-16 11:33 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
late-errno.diff hfuru, 2010-11-08 08:48 errno patch review
late_errno.patch pitrou, 2011-01-05 19:20 review
Messages (6)
msg120719 - (view) Author: Hallvard B Furuseth (hfuru) Date: 2010-11-08 08:48
errno is sometimes read too late after the error:  After another call
may have modified it.  Here's a patch against py3k.  Most of it or a
variant applies to 2.7 too, but I haven't really looked at that.

I've not looked at math code, where I don't even know what sets errno.

There might also be a late errno read at Modules/_ctypes/callproc.c
line 794: "space[0] = errno;".  Does it want the errno from before
_ctypes_get_errobj()?  I'm unsure what's going on there.

Modules/_multiprocessing/semaphore.c doesn't need the patch's extra
variable, it can instead be rearranged with the commented-out patch.
msg121062 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-11-12 19:31
This stuff is hard to write automated tests for, hence there are none.

The patch is mostly straightforward: capture errno with new variable err at point of possible error when intervening calculation is needed before testing the value of errno. This seems like a good idea.

There is one relocation of memory freeing and the additions of
+        if (res >= 0)
+            break;
which I cannot evaluate.
msg121218 - (view) Author: Hallvard B Furuseth (hfuru) Date: 2010-11-15 08:59
Terry J. Reedy writes:
> There is one relocation of memory freeing

Modules/timemodule.c does '#if,if(..errno..)' after PyMem_Free(outbuf),
which can overwrite the desired errno.  Instead of reading errno into
a temporary, I moved the free into both branches taken by the #if,if().

> and the additions of
> +        if (res >= 0)
> +            break;
> which I cannot evaluate.

errno is only needed after an error., so I moved 'res < 0' out of the
'while'.
        if (res == MP_EXCEPTION_HAS_BEEN_SET) break;
    } while (res < 0 && errno == EINTR && !PyErr_CheckSignals());
-->
        if (res == MP_EXCEPTION_HAS_BEEN_SET) break;
        if (! (res < 0))                      break;
    } while (errno == EINTR && !PyErr_CheckSignals());
-->
        if (res >= 0)                         break;
        err = errno;
        if (res == MP_EXCEPTION_HAS_BEEN_SET) break;
    } while (err == EINTR && !PyErr_CheckSignals());
msg125456 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-05 19:20
The patch seems a bit confused at times. For example, you need to restore errno before calling PyErr_SetFromErrno().
Here is a new patch for py3k.
msg149614 - (view) Author: Roundup Robot (python-dev) Date: 2011-12-16 11:32
New changeset 6a966179c73a by Antoine Pitrou in branch '3.2':
Issue #10350: Read and save errno before calling a function which might overwrite it.
http://hg.python.org/cpython/rev/6a966179c73a

New changeset 8e0b2e75ca7a by Antoine Pitrou in branch 'default':
Issue #10350: Read and save errno before calling a function which might overwrite it.
http://hg.python.org/cpython/rev/8e0b2e75ca7a
msg149615 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-16 11:33
Committed in 3.x. I won't bother backporting to 2.x. Thank you!
History
Date User Action Args
2011-12-16 11:33:09pitrousetstatus: open -> closed
versions: + Python 3.3, - Python 3.1, Python 2.7
messages: + msg149615

resolution: fixed
stage: patch review -> resolved
2011-12-16 11:32:40python-devsetnosy: + python-dev
messages: + msg149614
2011-01-05 19:25:31pitrousetnosy: + amaury.forgeotdarc
2011-01-05 19:20:11pitrousetfiles: + late_errno.patch
nosy: + pitrou
messages: + msg125456

2010-11-15 08:59:22hfurusetmessages: + msg121218
2010-11-12 19:31:56terry.reedysetnosy: + terry.reedy

messages: + msg121062
stage: patch review
2010-11-08 13:23:08giampaolo.rodolasetnosy: + giampaolo.rodola
2010-11-08 08:48:17hfurucreate