classification
Title: A file is not properly closed by webbrowser._invoke
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: anton.barkovsky, python-dev, r.david.murray, rosslagerwall
Priority: normal Keywords: patch

Created on 2012-07-25 13:15 by anton.barkovsky, last changed 2012-09-03 16:55 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
fileclose.patch anton.barkovsky, 2012-07-25 13:15 Close the file explicitly review
fileclose_devnull.patch anton.barkovsky, 2012-07-25 14:01 Use DEVNULL constant instead of manually opening a file review
fileclose_devnull_v2.patch anton.barkovsky, 2012-07-30 14:07 review
Messages (12)
msg166392 - (view) Author: Anton Barkovsky (anton.barkovsky) * Date: 2012-07-25 13:15
webbrowser._invoke opens /dev/null, never closes it and a warning is
printed.

I'm attaching a patch.
The diff looks messy, but I'm just wrapping the code in a try-finally
block, the rest is just indented.
msg166394 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-07-25 13:31
Thanks.  Is this warning printed by the webbrowser unit tests?  If not can you see a way to add one that does?
msg166395 - (view) Author: Anton Barkovsky (anton.barkovsky) * Date: 2012-07-25 13:38
The warning is printed by the file object when it closes itself in __del__:

  ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='r+' encoding='UTF-8'>

There isn't much to test, or is there?
msg166396 - (view) Author: Anton Barkovsky (anton.barkovsky) * Date: 2012-07-25 13:39
To clarify, I discovered this when I was simply running webbrowser.open
in REPL.
msg166397 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2012-07-25 13:43
Are there any webbrowser unit tests?

(this could probably use the new subprocess.DEVNULL constant in 3.3)
msg166399 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-07-25 13:48
@Anton: That's what I was guessing.  If we had a unit test in test_webbrowser that did the same thing, we'd have seen the resource warning when running the tests and fixed it.  However, it looks like there aren't *any* tests for webbrowser, not even in test_sundry (which just makes sure modules without tests are importable).

So adding a test that will trigger this resource warning requires setting up a test_webbrowser file first, even before we get to the problem of how to test something that wants to start up a web browser...(but that should be solvable with unittest.mock, I think).
msg166401 - (view) Author: Anton Barkovsky (anton.barkovsky) * Date: 2012-07-25 14:01
Adding a patch that uses subprocess.DEVNULL instead.

Writing tests for webbrowser should be a separate issue, right?
msg166404 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-07-25 14:27
You could do it either way.  Normally we prefer to have a test along with any fix; in this case adding a test involves adding the test module as well, but it is not different in principle.  If you want to work on it and prefer to have it as a separate issue that's fine, we'll just make the test issue dependent on this one.
msg166900 - (view) Author: Anton Barkovsky (anton.barkovsky) * Date: 2012-07-30 14:07
An updated patch with the same issue fixed in Konqueror class.
msg167424 - (view) Author: Anton Barkovsky (anton.barkovsky) * Date: 2012-08-04 17:54
Added tests in #15557.
msg169774 - (view) Author: Roundup Robot (python-dev) Date: 2012-09-03 16:53
New changeset 901c790e4417 by R David Murray in branch 'default':
#15447: Use subprocess.DEVNULL in webbrowser, instead of opening
http://hg.python.org/cpython/rev/901c790e4417

New changeset 5da3b2df38b3 by R David Murray in branch 'default':
#15557,#15447,#15509: webbrowser test suite added.
http://hg.python.org/cpython/rev/5da3b2df38b3
msg169779 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-09-03 16:55
Thanks, Anton.
History
Date User Action Args
2012-09-03 16:55:23r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg169779

stage: resolved
2012-09-03 16:53:25python-devsetnosy: + python-dev
messages: + msg169774
2012-08-09 13:22:11asvetlovlinkissue15557 dependencies
2012-08-04 17:54:40anton.barkovskysetmessages: + msg167424
2012-07-30 14:07:18anton.barkovskysetfiles: + fileclose_devnull_v2.patch

messages: + msg166900
2012-07-25 14:27:52r.david.murraysetmessages: + msg166404
2012-07-25 14:01:37anton.barkovskysetfiles: + fileclose_devnull.patch

messages: + msg166401
2012-07-25 13:48:16r.david.murraysetmessages: + msg166399
2012-07-25 13:43:08rosslagerwallsetnosy: + rosslagerwall
messages: + msg166397
2012-07-25 13:39:10anton.barkovskysetmessages: + msg166396
2012-07-25 13:38:04anton.barkovskysetmessages: + msg166395
2012-07-25 13:31:48r.david.murraysetnosy: + r.david.murray
messages: + msg166394
2012-07-25 13:15:13anton.barkovskycreate