Issue 28387: double free in io.TextIOWrapper

We have found that you can produce a crash when an instance of _io.TextIOWrapper is being deallocated while there's another thread invoking the garbage collector. I've attached a simple script that should reproduce the issue (textiowrapper_crash.py)

Looking at the code of the _io module, we found that on the dealloc method of the TextIOWrapper class, it first tries to invoke the close method, then releases its internal members, after that removes itself from the garbage collector tracking and finally frees deallocates the remaining stuff. What happens, is that while releasing it's internal members, it might end up calling code that releases the interpreter lock (because its doing an operating system call), letting other threads execute. If for example the thread that comes in, invokes the garbage collector, on debug will raise an assert on gcmodule.c on line 351, where I understand it is complaining that it is tracking an object with refcount 0. In a release build, I suppose this goes on and will end up releasing the object (given it has a refcount of 0), and when the interrupted dealloc thread continues will end up freeing again itself which at some point produces a crash.

Attached is a proposed fix for the issue in textio.c.patch, where the it the call to _PyObject_GC_UNTRACK is now done right after the call to the close method and before release its internal members.

As a reference we have been able to reproduce this with Python 2.7.12 on Windows (i386)

Thank you for your report and patch Sebastian.

In Python 3 the solution can be simpler, just move the line "_PyObject_GC_UNTRACK(self);" above the line "textiowrapper_clear(self);". But calling PyObject_ClearWeakRefs() also should be moved up. Otherwise half-destroyed TextIOWrapper instance can be accessed via weak references. Following patches do this (and small refactoring).

New changeset 91f024fc9b3a by Serhiy Storchaka in branch '2.7':
Issue #28387: Fixed possible crash in _io.TextIOWrapper deallocator when
https://hg.python.org/cpython/rev/91f024fc9b3a

New changeset 89f7386104e2 by Serhiy Storchaka in branch '3.5':
Issue #28387: Fixed possible crash in _io.TextIOWrapper deallocator when
https://hg.python.org/cpython/rev/89f7386104e2

New changeset c4319c0d0131 by Serhiy Storchaka in branch '3.6':
Issue #28387: Fixed possible crash in _io.TextIOWrapper deallocator when
https://hg.python.org/cpython/rev/c4319c0d0131

New changeset 36af3566b67a by Serhiy Storchaka in branch 'default':
Issue #28387: Fixed possible crash in _io.TextIOWrapper deallocator when
https://hg.python.org/cpython/rev/36af3566b67a

messages: + msg279717
versions: + Python 3.5, Python 3.6, Python 3.7

components: + IO
stage: patch review