BIND 10 #2410: race condition(?) in dhcp[46]_test.py

[BIND 10 Development]

#2410: race condition(?) in dhcp[46]_test.py
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  stephen
                       Type:         |                Status:  reviewing
  defect                             |             Milestone:  Sprint-
                   Priority:         |  DHCP-20121129
  medium                             |            Resolution:
                  Component:  dhcp   |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DHCP
            Defect Severity:  N/A    |  Estimated Difficulty:  4
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by marcin):

 * owner:  marcin => stephen


Comment:

 Reviewed commit 56bcca870b41742f47160cae5b30b81ef62c1095

 It seems to me that in the future we could change os.spawn approach to
 subprocess.communicate as it is intended to replace old modules. That
 would also help to avoid race condition.
 Do we want to submit the ticket on this one?

 '''dhcp6_test.py'''
 The code appears to have some redundancy. There are two variables:
 '''output''' and '''new_output'''. The latter one receives new chunks of
 output from a process and appends it to the other variable. When process
 finally produces the output both variables are empty initially. When the
 following happens:
 {{{
 if (new_output is not None):
     output = output + str(new_output)
 }}}
 '''output''' is equal to '''new_output'''.
 We finally hit this check:
 {{{
 if pi.process.poll() is not None or len(error) > 0 or len(output) > 0:
     break;
 }}}
 Let's assume that process is still running, there were no errors. The
 output is not empty (it is equal to new_output) so the loop breaks and it
 never gets to the point when the next chunk of process'es output is read
 to '''new_output''' and thus no other chunk of data is appended to
 '''output'''. What is then the reason to keep two variables around? Why
 don't we just write to '''output''' directly?

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