Title: testcase for exception binhex.Error
Type: Stage: resolved
Components: Library (Lib), Tests Versions: Python 2.7
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: SilentGhost, arkady.koplyarov, brian.curtin, ncoghlan, python-dev
Priority: normal Keywords: patch

Created on 2011-03-16 19:36 by arkady.koplyarov, last changed 2011-03-17 14:46 by ncoghlan. This issue is now closed.

File name Uploaded Description Edit
testcase_for_binhex_module.patch arkady.koplyarov, 2011-03-16 19:36 review
testcase_for_binhex_module_2.patch arkady.koplyarov, 2011-03-16 22:11 review
Messages (8)
msg131159 - (view) Author: Arkady Koplyarov (arkady.koplyarov) * Date: 2011-03-16 19:36
Testcase for exception binhex.Error to increase test coverage.
msg131163 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-03-16 19:46
unittest provides a utility to help with testing that correct exceptions are raised:
msg131183 - (view) Author: Arkady Koplyarov (arkady.koplyarov) * Date: 2011-03-16 22:11
The testcase updated accordingly to Nick Coghlan's suggestion to use assertRaises() since the testcase deals with an exception.
msg131189 - (view) Author: Arkady Koplyarov (arkady.koplyarov) * Date: 2011-03-16 22:46
The testcase provided shows up a resource leakage:
C:\_cpython\cpython>PCbuild\python_d.exe  -m test.regrtest test_binhex
[1/1] test_binhex
C:\_cpython\cpython\lib\unittest\ ResourceWarning: unclosed file <_io.BufferedWriter name='@test_5592_tmp2'>
  callableObj(*args, **kwargs)
1 test OK.
The resource leakage occurs in module in binhex(inp,out) > BinHex.__init__() > _writeinfo() when the exception binhex.Error is raised in _writeinfo() at the code line:
    raise Error('Filename too long')
The issue is that when the exception is thrown the file is left unclosed.

One of possible fixes is to catch the thrown exception and close the unclosed file in the BinHex.__init__().
msg131191 - (view) Author: SilentGhost (SilentGhost) * Date: 2011-03-16 22:53
Arkady, I don't see why you need to catch exception. Just wrap the _writeinfo call into a try-finally block, and close ofp in the finally.
msg131201 - (view) Author: Arkady Koplyarov (arkady.koplyarov) * Date: 2011-03-17 00:25
Well, I believe that in BinHex.__init__() I cannot just wrap the _writeinfo call into a try-finally block, and close ofp in the finally.

I see that in the case of normal operation when the exception is not thrown, the output file descriptor ofp is used in binhex(inp,out) as a result of BinHex.__init__() and so need remain open until binhex(inp,out) close it explicitly.
msg131208 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-17 01:54
New changeset 5c2d15c6007e by Nick Coghlan in branch '3.2':
Close #11577: Improve binhex test coverage and fix ResourceWarning
msg131259 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-03-17 14:46
This is fixed on default as well, I just stuffed the merge so the history looks odd and the integration script didn't pick it up.

You can see the additional changes I made in the linked changeset:
- try/except with a flag to clean up implicitly created file objects when binhex.__init__ fails
- change the getfileinfo code to use a with statement so it is also more exception tolerant
Date User Action Args
2011-03-17 14:46:51ncoghlansetnosy: ncoghlan, brian.curtin, SilentGhost, python-dev, arkady.koplyarov
messages: + msg131259
2011-03-17 01:54:30python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg131208

resolution: fixed
stage: resolved
2011-03-17 00:25:44arkady.koplyarovsetnosy: ncoghlan, brian.curtin, SilentGhost, arkady.koplyarov
messages: + msg131201
2011-03-16 22:53:59SilentGhostsetnosy: + SilentGhost
messages: + msg131191
2011-03-16 22:46:37arkady.koplyarovsetnosy: ncoghlan, brian.curtin, arkady.koplyarov
messages: + msg131189
2011-03-16 22:11:36arkady.koplyarovsetfiles: + testcase_for_binhex_module_2.patch
nosy: ncoghlan, brian.curtin, arkady.koplyarov
messages: + msg131183
2011-03-16 19:46:22ncoghlansetnosy: ncoghlan, brian.curtin, arkady.koplyarov
messages: + msg131163
2011-03-16 19:40:17ncoghlansetnosy: + ncoghlan
2011-03-16 19:37:34brian.curtinsetnosy: + brian.curtin
2011-03-16 19:36:24arkady.koplyarovcreate