autotest/factory: Interrupt tests by factory UI.

autotest/factory: Interrupt tests by factory UI.

The original approach - sending USR1 to control and letting contol to
nuke the active test, may raise EINTR to any system I/O in main control,
which crashed the main control then the UI.

In the future we may change the communication channel to a file descriptor.
To minimize the impact to currect autotest framework, this CL let factory
UI to kill active tests directly.

NOTE: by that change, fork_wait_for won't raise OSError anymore so we
have to override entire function.

BUG=chrome-os-partner:2514
TEST=Verified on Alex that rapidly hitting hotkeys won't crash the UI
     (tested for about 30 seconds -- without this CL, UI usually crashed
      within 10 seconds)

Change-Id: I6cba16907e85d81cf8975c3fe6e6d684f0221848

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=5d49ebf

[Hung-Te, 2011-03-04 01:17:46]

Adding josephsih as reviewer because he also had experience in the killing
active test stuff.

[Nick Sanders, 2011-03-04 02:11:34]

lgtm

[josephsih1, 2011-03-04 02:12:29]

LGTM with some minor nits.

Two comments inside factory_ui.

In factory,py, it registers a signal handler kill_current_test_callback() to
handle USR1. Do we still need it?

On 2011/03/04 01:17:46, Hung-Te wrote:
> Adding josephsih as reviewer because he also had experience in the killing
> active test stuff.

[josephsih1, 2011-03-04 02:13:11]

http://codereview.chromium.org/6614014/diff/3001/client/bin/factory_ui
File client/bin/factory_ui (right):

http://codereview.chromium.org/6614014/diff/3001/client/bin/factory_ui#newcod...
client/bin/factory_ui:268: if active_test_data is None:
As in kill_current_test_callback() in factory.py, can we add a log as --
log('SIGUSR1 ... KILLING NOTHING, no active test') -- when there is no
active_test_data?

http://codereview.chromium.org/6614014/diff/3001/client/bin/factory_ui#newcod...
client/bin/factory_ui:272: utils.nuke_pid(active_test_data[1])
In the original design, the subprocess killing is executed using
parallel.fork_nuke_subprocess, which invokes the checking of
parallel._check_for_subprocess_exception(). In this way, you may not need to
invoke it explicitly in suite_Factory/control. Could we maintain this way of
subprocess killing too?

[Hung-Te, 2011-03-04 02:15:16]

On 2011/03/04 02:12:29, josephsih1 wrote:
> In factory,py, it registers a signal handler kill_current_test_callback() to
> handle USR1. Do we still need it?
   I'd prefer to keep it temporarily, for help testing / comparing the impact of
new change.
   User in command line can also use USR1 to force killing active tests -
although not suggested.

[Hung-Te, 2011-03-04 02:21:28]

http://codereview.chromium.org/6614014/diff/3001/client/bin/factory_ui
File client/bin/factory_ui (right):

http://codereview.chromium.org/6614014/diff/3001/client/bin/factory_ui#newcod...
client/bin/factory_ui:268: if active_test_data is None:
Actually we should (almost) never see this, because it's only possible in the
very very early (before any tests startup, or if the main is broken so no one is
running) of main.

Anyway I can put it back next time, thanks for the suggestion!

http://codereview.chromium.org/6614014/diff/3001/client/bin/factory_ui#newcod...
client/bin/factory_ui:272: utils.nuke_pid(active_test_data[1])
These are in different processes, so it would not do what we want - that will
raise exception in UI and leave nothing in main control :)