classification
Title: Make regrtest with --huntrleaks check for fd leaks
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.4, Python 3.5, Python 2.7
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: ezio.melotti, neologix, pitrou, python-dev, sbt, vstinner
Priority: normal Keywords: patch

Created on 2013-06-09 19:47 by sbt, last changed 2018-09-19 22:48 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
fdleak.patch sbt, 2013-06-09 19:47 review
fdleak.patch sbt, 2013-06-12 14:57 review
fdleak-2.patch vstinner, 2014-07-28 22:49 review
Pull Requests
URL Status Linked Edit
PR 7408 merged vstinner, 2018-06-04 22:23
PR 7409 merged vstinner, 2018-06-04 22:43
PR 7420 merged vstinner, 2018-06-05 11:19
PR 7827 closed vstinner, 2018-06-20 14:55
PR 7966 closed vstinner, 2018-06-27 14:42
Messages (25)
msg190871 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013-06-09 19:47
regrtest already tests for refcount leaks and memory allocation leaks.  It can also be made to check for file descriptor leaks (and perhaps also handles on Windows).

Running with the attached patch makes it look like test_openpty, test_shutil, test_subprocess, test_uuid all leak fds on Linux, but I have not investigated:

$ ./python -m test.regrtest -R 3:3 test_openpty test_shutil test_subprocess test_uuid
[1/4] test_openpty
123456
......
test_openpty leaked [2, 2, 2] fds, sum=6
[2/4/1] test_shutil
beginning 6 repetitions
123456
......
test_shutil leaked [4, 4, 4] fds, sum=12
[3/4/2] test_subprocess
beginning 6 repetitions
123456
......
test_subprocess leaked [5, 5, 5] fds, sum=15
[4/4/3] test_uuid
beginning 6 repetitions
123456
......
test_uuid leaked [1, 1, 1] fds, sum=3
4 tests failed:
    test_openpty test_shutil test_subprocess test_uuid
msg190915 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-06-10 15:33
New changeset a7381fe515e8 by Richard Oudkerk in branch '2.7':
Issue #18174: Fix fd leaks in tests.
http://hg.python.org/cpython/rev/a7381fe515e8

New changeset 46fe1bb0723c by Richard Oudkerk in branch '3.3':
Issue #18174: Fix fd leaks in tests.
http://hg.python.org/cpython/rev/46fe1bb0723c
msg190918 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013-06-10 15:53
The test_shutil leak is caused by #17899.  The others are fixed by  a7381fe515e8 and 46fe1bb0723c.
msg191031 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013-06-12 14:57
Updated version which adds checks for handle leaks on Windows.
msg223790 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-23 22:29
Just flagging this up as testing is rather important.
msg223874 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-07-24 19:19
The patch looks good.
I just think it would be nice to expose _fdcount() in test.support.
msg223881 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-24 19:50
Why not using os.fstat() instead of os.dup() to check if a file descriptor is open or not?

It's strange to create a new file descriptor with os.dup() to count the file descriptors.
msg223882 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-07-24 19:53
> Why not using os.fstat() instead of os.dup() to check if a file descriptor is open or not?

I asked myself the same question, but IIRC, fstat() doesn't always
work on Windows (of course).
msg223885 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-24 19:57
> I asked myself the same question, but IIRC, fstat() doesn't always
work on Windows (of course).

Can you please elaborate?
msg223887 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-07-24 20:05
> STINNER Victor added the comment:
>
>> I asked myself the same question, but IIRC, fstat() doesn't always
> work on Windows (of course).
>
> Can you please elaborate?

Not really, since I don't know much about Windows, but that's
something I think I heard.
Richard will be able to give more details.
msg224192 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2014-07-28 21:43
I can't remember why I did not use fstat() -- probably it did not occur to me.
msg224200 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-28 22:49
Modified patch to use os.fstat(), and try to use /proc/self/fd/ on Linux.
msg224201 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-07-28 23:01
New changeset 379aad232000 by Victor Stinner in branch '3.4':
Issue #11453, #18174: Fix leak of file descriptor in test_asyncore
http://hg.python.org/cpython/rev/379aad232000

New changeset 0ced2d2325fb by Victor Stinner in branch 'default':
(Merge 3.4) Issue #11453, #18174: Fix leak of file descriptor in test_asyncore
http://hg.python.org/cpython/rev/0ced2d2325fb
msg224203 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-07-28 23:15
New changeset 746339776f19 by Victor Stinner in branch '3.4':
Issue #18174: Fix leak of file descriptor in test_tempfile
http://hg.python.org/cpython/rev/746339776f19

New changeset 017d701116d5 by Victor Stinner in branch 'default':
(Merge 3.4) Issue #18174: Fix leak of file descriptor in test_tempfile
http://hg.python.org/cpython/rev/017d701116d5
msg224236 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-07-29 17:29
> Richard Oudkerk added the comment:
>
> I can't remember why I did not use fstat() -- probably it did not occur to me.

I probably have Alzeihmer, I was sure I heard a reasonable case for
dup() vs fstat().
The only thing I can think of is that fstat() can incur I/O, whereas
dup() shouldn't.

In any case, I too prefer fstat(), since it doesn't require creating a
new FD (which could fail with EMFILE/ENFILE), and is more logical.

The only comment I have about this patch is I think that the helper
function to detect the number of open FD might have its place in
test.support.
msg224239 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-07-29 18:31
fstat() can do I/O, or can fail for other reasons. If you don't want to create a new fd, I think you can do dup2(fd, fd).

I don't understand the reason for the following code:

+    def check_handle_deltas(deltas):
+        return abs(sum(deltas)) >= min(3, len(deltas))

Can you add a comment?
msg224241 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-29 19:57
> fstat() can do I/O, or can fail for other reasons.

Oh, I forgot this reason. Maybe we should add a comment to explain that. I mean in the patch for this issue but also in is_valid_fd() (Python/pythonrun.c).
msg251849 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-09-29 12:00
New changeset 0e7d71a3bf0d by Victor Stinner in branch 'default':
Issue #18174: Explain why is_valid_fd() uses dup() instead of fstat()
https://hg.python.org/cpython/rev/0e7d71a3bf0d
msg252169 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-02 22:47
New changeset 72129c767c92 by Victor Stinner in branch 'default':
Issue #18174: "python -m test --huntrleaks ..." now also checks for leak of
https://hg.python.org/cpython/rev/72129c767c92
msg252172 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-02 22:49
I commited the first part to check for file descriptor leak.

I didn't commit the second part, to check for Windows handle leak.
msg252183 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-03 00:22
New changeset ec2ef7525fa5 by Victor Stinner in branch 'default':
Issue #18174: Fix test_regrtest when Python is compiled in release mode
https://hg.python.org/cpython/rev/ec2ef7525fa5
msg318720 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-04 22:36
New changeset 270581905cab2747ae8f7ee945301d6a29509cc7 by Victor Stinner in branch '2.7':
bpo-18174: Fix file descriptor leaks in tests (GH-7408)
https://github.com/python/cpython/commit/270581905cab2747ae8f7ee945301d6a29509cc7
msg318732 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-05 10:36
New changeset 64856ad8b7279718ff10a9fb32003c2221af7228 by Victor Stinner in branch '2.7':
bpo-18174: regrtest -R 3:3 now also detects FD leak (#7409)
https://github.com/python/cpython/commit/64856ad8b7279718ff10a9fb32003c2221af7228
msg318735 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-05 11:30
New changeset bc3df70b266304f78ebe5eabead71cabd4738d12 by Victor Stinner in branch '2.7':
bpo-18174: Fix fd_count() on FreeBSD (GH-7420)
https://github.com/python/cpython/commit/bc3df70b266304f78ebe5eabead71cabd4738d12
msg325815 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-09-19 22:48
I give up on that close. I closed my PR 7827 and PR 7966.
History
Date User Action Args
2018-09-19 22:48:53vstinnersetstatus: open -> closed
resolution: wont fix
messages: + msg325815

stage: patch review -> resolved
2018-06-27 14:42:05vstinnersetpull_requests: + pull_request7575
2018-06-20 14:55:39vstinnersetpull_requests: + pull_request7434
2018-06-05 11:30:51vstinnersetmessages: + msg318735
2018-06-05 11:19:28vstinnersetpull_requests: + pull_request7045
2018-06-05 10:36:35vstinnersetmessages: + msg318732
2018-06-05 02:11:49BreamoreBoysetnosy: - BreamoreBoy
2018-06-04 22:43:38vstinnersetpull_requests: + pull_request7035
2018-06-04 22:36:44vstinnersetmessages: + msg318720
2018-06-04 22:23:20vstinnersetpull_requests: + pull_request7034
2016-01-01 04:28:11ezio.melottisettype: enhancement
stage: patch review
2015-10-03 00:22:28python-devsetmessages: + msg252183
2015-10-02 22:49:56vstinnersetmessages: + msg252172
2015-10-02 22:47:44python-devsetmessages: + msg252169
2015-09-29 12:00:14python-devsetmessages: + msg251849
2014-07-29 19:57:26vstinnersetmessages: + msg224241
2014-07-29 18:31:43pitrousetnosy: + pitrou
messages: + msg224239
2014-07-29 17:29:39neologixsetmessages: + msg224236
2014-07-28 23:15:56python-devsetmessages: + msg224203
2014-07-28 23:01:59python-devsetmessages: + msg224201
2014-07-28 22:49:38vstinnersetfiles: + fdleak-2.patch

messages: + msg224200
2014-07-28 21:43:07sbtsetmessages: + msg224192
2014-07-24 20:05:30neologixsetmessages: + msg223887
2014-07-24 19:57:20vstinnersetmessages: + msg223885
2014-07-24 19:53:44neologixsetmessages: + msg223882
2014-07-24 19:50:03vstinnersetmessages: + msg223881
2014-07-24 19:19:12neologixsetnosy: + neologix
messages: + msg223874
2014-07-23 22:29:58BreamoreBoysetversions: + Python 2.7, Python 3.4, Python 3.5
nosy: + BreamoreBoy

messages: + msg223790

components: + Tests
2013-06-15 21:14:07ezio.melottisetnosy: + ezio.melotti
2013-06-12 16:36:46vstinnersetnosy: + vstinner
2013-06-12 14:57:07sbtsetfiles: + fdleak.patch

messages: + msg191031
2013-06-10 15:53:50sbtsetmessages: + msg190918
2013-06-10 15:33:14python-devsetnosy: + python-dev
messages: + msg190915
2013-06-09 19:47:53sbtcreate