io.FileIO('foo', 'rt') prints a RuntimeWarning

[amauryfa]

BPO	4237
Nosy	@warsaw, @amauryfa, @vstinner, @tiran
Files	issue_4237.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/tiran'
closed_at = <Date 2008-10-30.22:03:14.904>
created_at = <Date 2008-10-30.01:02:18.138>
labels = ['interpreter-core', 'release-blocker', 'performance']
title = "io.FileIO('foo', 'rt') prints a RuntimeWarning"
updated_at = <Date 2008-10-30.22:03:14.903>
user = 'https://github.com/amauryfa'

bugs.python.org fields:

activity = <Date 2008-10-30.22:03:14.903>
actor = 'christian.heimes'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2008-10-30.22:03:14.904>
closer = 'christian.heimes'
components = ['Interpreter Core']
creation = <Date 2008-10-30.01:02:18.138>
creator = 'amaury.forgeotdarc'
dependencies = []
files = ['11909']
hgrepos = []
issue_num = 4237
keywords = ['patch', 'needs review']
message_count = 9.0
messages = ['75346', '75349', '75350', '75351', '75357', '75370', '75375', '75382', '75386']
nosy_count = 5.0
nosy_names = ['barry', 'amaury.forgeotdarc', 'vstinner', 'christian.heimes', 'LambertDW']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue4237'
versions = ['Python 2.6']

[amauryfa]

>>> import io
>>> io.FileIO('foo', 'rt')
__main__:1: RuntimeWarning: Trying to close unclosable fd!
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid mode: rt

The ValueError is expected, but the warning is not.
This happens on file deallocation: the file object is in an invalid
state.

[tiran]

Verified. The issue should be easy to fix.

I wonder why 'rt' is an invalid mode. Python 2.x supports 'rt'.

[tiran]

I propose to move self->closefd closer to the top of the function:

Index: Modules/_fileio.c
===================================================================
--- Modules/_fileio.c (Revision 67040)
+++ Modules/_fileio.c (Arbeitskopie)
@@ -181,6 +181,7 @@

        self->readable = self->writable = 0;
        self->seekable = -1;
+       self->closefd = closefd;
        s = mode;
        while (*s) {
                switch (*s++) {
@@ -243,7 +244,6 @@

        if (fd >= 0) {
                self->fd = fd;
-               self->closefd = closefd;
        }
        else {
                self->closefd = 1;

[lambertdw]

>>> print(io.read.__doc__)
    ...
    The default mode is 'rt' (open for reading text).
    ...

[amauryfa]

'rt' is a valid mode for open() or io.open().
But io.FileIO is the unbuffered raw binary file...

Python 2.6 has exactly the same problem and displays the same
RuntimeWarning.

Christian, the change you propose does not fix another case:
>>> io.FileIO([])

[tiran]

The new patch fixes the problem and adds a unit test, too.

The bug was caused by a design flaw -- which was partly my fault. Some
elements of the PyFileIOObject struct were initialized in __new__ while
other parts were initialized in __init__. I've moved the initialization
to __new__.

We should add a rule that all struct members must be properly
initialized in __new__. In the past Victor's fuzzying tool has revealed
several crashers related to similar design flaws.

I'm raising the severity of the bug to release blocker because I can't
predict if the problem can be abused to crash the interpreter. We should
also review all __new__ and __init__ methods of objects and extension
modules for similar issues.

[amauryfa]

The patch looks fine to me

[tiran]

Fixed in r67051 (py3k), r67052 (trunk)

[tiran]

Merged into the release26 branch, too.