BIND 10 #1828: Python tests print warnings about unclosed file handles / sockets

[BIND 10 Development]

#1828: Python tests print warnings about unclosed file handles / sockets
-------------------------------------+-------------------------------------
                   Reporter:  muks   |                 Owner:  UnAssigned
                       Type:         |                Status:  reviewing
  defect                             |             Milestone:
                   Priority:         |  Sprint-20120417
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  Unclassified                       |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 I've not fully reviewed the code yet, but have some initial comments:

 First off, do we really need things like this?
 {{{#!diff
 -        open(self.pid_file, "w").write('dummy data\n')
 +        fd = open(self.pid_file, "w")
 +        fd.write('dummy data\n')
 +        fd.close()
 }}}
 As Shane mentioned in #845, this is very counter intuitive, and if we
 really need to explicitly close opened files this way, it seems to me
 something is very wrong.  At the very least I don't think we can
 reliably keep this convention in code we'll develop in future (except
 by seeing the warnings and modifying the code to silence them).

 I've written the following simple test code:

 {{{#!python
 while True:
     open('f.py', 'r')
 }}}

 and ran it with 'python -W default'.  It produced the warning, but
 otherwise it just kept (busy) running, suggesting the temporary file
 object
 is closed at the end of the loop:

 {{{
 % python3.2 -W default f.py
 f.py:4: ResourceWarning: unclosed file <_io.TextIOWrapper name='f.py'
 mode='r' encoding='UTF-8'>
   open('f.py', 'r')
 (no other disruption)
 }}}

 So, I suspect there are only a small number of real leak (or very delayed
 close), and the others are false alarm.  I also suspect this warning
 is quite naive and just performs simple syntax level checks (e.g.,
 whether explicit close() follows an open()).  While seeing too noisy
 warning is not a good thing, I don't like to tweak our code in a
 counter intuitive manner just to silence the warning.

 My suggestion is to identify the real leak and fix them, and leave
 others open, and, ideally, suppress this warning (in general I don't
 like to suppress warning, but in this specific case I think it does
 more harm than good).

 See also this: http://projects.scipy.org/scipy/ticket/1385

 One other minor comment at this moment: for the real (few, I suspect)
 cases we need to store the result of open() in a variable, 'fd'
 doesn't seem to be a good name because the return value is a 'file
 object', not a descriptor.  I'd use something like 'fobj', 'file', etc.

-- 
Ticket URL: <http://bind10.isc.org/ticket/1828#comment:9>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development