If TextIOWrapper constructor fails in tokenize.open(), the open binary file is not closed and a ResourceWarning is emited.

Try attached patch:

$ python3 -Wd ~/tokenize_open_bug.py
tokenize.open failed: unknown encoding for 'test.py': xxx
/home/haypo/tokenize_open_bug.py:13: ResourceWarning: unclosed file <_io.BufferedReader name='test.py'>
  print("tokenize.open failed: %s" % err)

The fix is quite simple: add "try: ... except: buffer.close(); raise". If someone wants to fix this issue, an unit test must be added, test based on my script and ensures that the buffer is closed (ex: mock tokenize._builtin_open and checks that close() was called).

I sent an email to the python core mentorship mailing list to find a volunteer to fix this easy issue.

ITSM it's not the TextIOWrapper but the detect_encoding fails and throws an error.

So I wrapped a try/catch block around that.

Comparing to introom. Any reason to not wrap the entire in a try? Attached patch.

Curious,
@Haypo, are you looking for a new unittest or a function within an existing unit test? Perhaps go under TestTokenize?

1227 def test_main():
1228 from test import test_tokenize
1229 support.run_doctest(test_tokenize, True)
1230 support.run_unittest(TestTokenizerAdheresToPep0263)
1231 support.run_unittest(Test_Tokenize)
1232 support.run_unittest(TestDetectEncoding)
1233 support.run_unittest(TestTokenize)
1234 support.run_unittest(UntokenizeTest)

I believe it goes here under the TestTokenize class. Going to give it a shot now.

ITSM it's not the TextIOWrapper but the detect_encoding fails and throws an error.

Oh, right. But TextIOWrapper can fail for differen reasons. For example, CTRL+c may send KeyboardInterrupt.

Try for example:

    with unittest.mock.patch.object(tokenize, '_builtin_open') as mock_open:
        mock_file = mock_open.return_value
        mock_file.tell.side_effect = OSError
        mock_file.readline.return_value = b''

        tokenize.open(fn)

This example raises an OSError in TextIOWrapper on file.tell(), and file.close() is not called.

On Wed, Apr 1, 2015 at 6:18 PM, STINNER Victor <report@bugs.python.org> wrote:

Oh, right. But TextIOWrapper can fail for differen reasons. For example, CTRL+c may send KeyboardInterrupt.

Yes. Any comment on the test?

I wil update a second patch.

Regards.

--

吾輩は猫である。ホームーページはhttp://introo.me。

Yes. Any comment on the test?

Yes, as I wrote, I reviewed patches. You may get a notification by email. Anyway, it's here:
http://bugs.python.org/review/23840/diff/14417/Lib/test/test_tokenize.py
(I also commented tokenize.py)

Updated patch based off of haypo's comment. Changed "except Exception as err:" -> "except"

Based upon the previous review.

Catch the TextWrapper, move the test into a function.

Based off of storchaka's comment, moving the

tokenize.patch is wrong: you should not call buffer.close() if TextIOWrapper() was created successfully.

tokenizeV2.patch and tokenize.patch have issues, I reviewed them. Can someone please write a new patch taking my comments in account?

Hey Haypo,
I'm working on submitting the new patch now. Still getting used to the workflow and tools here. Thanks for being patient.

You should see the new file in the next 30 minutes.

You should see the new file in the next 30 minutes.

I don't see the new file.

New changeset 623e07ea43df by Victor Stinner in branch '3.4':
Issue bpo-23840: tokenize.open() now closes the temporary binary file on error to
https://hg.python.org/cpython/rev/623e07ea43df

New changeset a640d268ba97 by Victor Stinner in branch '3.5':
(Merge 3.5) Issue bpo-23840: tokenize.open() now closes the temporary binary file
https://hg.python.org/cpython/rev/a640d268ba97

New changeset c5cfd6353b4b by Victor Stinner in branch 'default':
(Merge 3.6) Issue bpo-23840: tokenize.open() now closes the temporary binary file
https://hg.python.org/cpython/rev/c5cfd6353b4b

Sorry for not being more available for feedback on patches. I chose to write the final patch because patches were not updated to take in account my latest comments.

I hope that this issue helps you at least the process for reviewing patches ;-)