752951 - if we timeout ttest.py, bcontroller.py can still hang around

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

it looks like we can hit a condition where bcontroller.py hangs and the talos test terminates.  In that scenario, we need to kill the process if it exists in our error handling and cleanup routines.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: kill bcontroller.py at the end of talos. (1.0)

Comment 2

[William Lachance (:wlach)]

Comment on attachment 622033 [details] [diff] [review]
kill bcontroller.py at the end of talos. (1.0)

Review of attachment 622033 [details] [diff] [review]:
-----------------------------------------------------------------

The idea is right, but IMO we should just call kill() on the Popen object that represents bcontroller, instead of using talos's abstractions. Less code, easier to understand.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: kill bcontroller.py at the end of talos. (2.0)

much simpler.

Comment 4

[William Lachance (:wlach)]

Comment on attachment 622052 [details] [diff] [review]
kill bcontroller.py at the end of talos. (2.0)

No need for the "if process:". There is no circumstance where process could be None. Other than that, looks fine.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

http://hg.mozilla.org/build/talos/rev/8ee9b373ea24

Comment 6

[Jeff Hammel]

Note that on python 2.4 (which we still use for windows) there is no Popen.kill method:

Traceback (most recent call last):
  File "run_tests.py", line 640, in ?
    main()
  File "run_tests.py", line 637, in main
    test_file(arg, options, parser.parsed)
  File "run_tests.py", line 574, in test_file
    browser_dump, counter_dump, print_format = mytest.runTest(browser_config, test)
  File "c:\talos-slave\talos-data\talos\ttest.py", line 343, in runTest
    process.kill()
AttributeError: 'Popen' object has no attribute 'kill'

We should not deploy this until this is fixed.  Probably the thing to do is

try:
   os.kill(process.pid, signal.SIGKILL)
except:
   # do something!

Comment 7

[Jeff Hammel]

I can take this as a ridealong to bug 704654 if desired

Comment 8

[Jeff Hammel]

actually since we don't want to guess or rely on what os.kill does on windows, and we don't really care about windows which is the only case we have py 2.4, maybe just checking if .kill exists is sufficient:

if hasattr(process, 'kill'):
   process.kill()

This line of code also seems to be the problem with https://bugzilla.mozilla.org/show_bug.cgi?id=753526 although that is mac.  If I kill processes that don't exist on linux, i get no errors

Comment 9

[Jeff Hammel]

in addition you can get oserror no such process when doing this. in linux and mac this is errno 3. in windows im not sure how/if this shows itself.

Comment 10

[Jeff Hammel]

should be fixed with bug 704654: http://hg.mozilla.org/build/talos/rev/1680be7af3bf#l11.13

It'd be nice to have a better solution for this, but I think/hope this will do for now.  mozprocess ftw