Add support for killing a process with a given PID to dart:io

Add support for killing a process with a given PID to dart:io

R=lrn@google.com, kustermann@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=43455

[Lasse Reichstein Nielsen, 2015-02-03 14:00:48]

LGTM (only looked at Dart code).

https://chromiumcodereview.appspot.com/895623002/diff/1/sdk/lib/io/process.dart
File sdk/lib/io/process.dart (right):

https://chromiumcodereview.appspot.com/895623002/diff/1/sdk/lib/io/process.da...
sdk/lib/io/process.dart:349: * On Linux and Mac OS, [killPid] sends [signal] to
the process with
Is it officially called Mac OS? Or is it just OS X? (Wikipedia says it was
renamed to just OS X in 2012)

https://chromiumcodereview.appspot.com/895623002/diff/1/sdk/lib/io/process.da...
sdk/lib/io/process.dart:353: * `signal` flag.
Start with a single statement describing the operation with no conditionals or
details, e.g.:
  Kills the process with id `pid`.
Then heap on the details. If a summary of the API only shows the first line,
then it might look like this function only works on OS/X and Linux.

This restricts us to those three three OSes (which is probably not just
happening here).

Could it be written as:

Where possible, sends the [signal] to the process with id [pid]. This includes
Linux and OS/X. The default signal is [ProcessSignal.SIGTERM] which will
terminate the process.

On platforms without signal support, including Windows, the call just kills the
process with id `pid` in a platform specific way, and the 
`signal` parameter is ignored.

Can't you send signals on Windows?
https://msdn.microsoft.com/en-us/library/ms811896.aspx#ucmgch09_topic3

https://chromiumcodereview.appspot.com/895623002/diff/1/sdk/lib/io/process.da...
sdk/lib/io/process.dart:355: * Returns `true` if the signal is successfully sent
and process is killed.
and process -> and the process

What if the process intercepts the SIGTERM signal? Does it return true in that
case (I guess yes), but the process isn't killed.

SO:
  Returns `true` if the signal is successfully sent.
but skip" process is killed" because it might not be.

https://chromiumcodereview.appspot.com/895623002/diff/1/tests/standalone/io/p...
File tests/standalone/io/process_kill_test.dart (right):

https://chromiumcodereview.appspot.com/895623002/diff/1/tests/standalone/io/p...
tests/standalone/io/process_kill_test.dart:26: // Start a process that will hang
waiting for input until killed.
You need to add startAsync/endAsync calls, otherwise the test succeedes before
it has time to fail.
Same goes for first test.

https://chromiumcodereview.appspot.com/895623002/diff/1/tests/standalone/io/p...
tests/standalone/io/process_kill_test.dart:32: Expect.isFalse(p.kill());
Why not Process.killPid(p.pid) ?

[Søren Gjesse, 2015-02-04 08:16:02]

https://chromiumcodereview.appspot.com/895623002/diff/1/sdk/lib/io/process.dart
File sdk/lib/io/process.dart (right):

https://chromiumcodereview.appspot.com/895623002/diff/1/sdk/lib/io/process.da...
sdk/lib/io/process.dart:349: * On Linux and Mac OS, [killPid] sends [signal] to
the process with
On 2015/02/03 14:00:47, Lasse Reichstein Nielsen wrote:
> Is it officially called Mac OS? Or is it just OS X? (Wikipedia says it was
> renamed to just OS X in 2012)

You are right. We are currently using Mac OS in the other files in the SDK, but
there are < 20 occurrences. I will change it here and make a CL for changing the
rest.

https://chromiumcodereview.appspot.com/895623002/diff/1/sdk/lib/io/process.da...
sdk/lib/io/process.dart:353: * `signal` flag.
On 2015/02/03 14:00:47, Lasse Reichstein Nielsen wrote:
> Start with a single statement describing the operation with no conditionals or
> details, e.g.:
>   Kills the process with id `pid`.
> Then heap on the details. If a summary of the API only shows the first line,
> then it might look like this function only works on OS/X and Linux.
> 
> This restricts us to those three three OSes (which is probably not just
> happening here).
> 
> Could it be written as:
> 
> Where possible, sends the [signal] to the process with id [pid]. This includes
> Linux and OS/X. The default signal is [ProcessSignal.SIGTERM] which will
> terminate the process.
> 
> On platforms without signal support, including Windows, the call just kills
the
> process with id `pid` in a platform specific way, and the 
> `signal` parameter is ignored.
> 
> Can't you send signals on Windows?
> https://msdn.microsoft.com/en-us/library/ms811896.aspx#ucmgch09_topic3

Thanks, rewritten.

The signals you can receive on Windows are only exceptions generated by the OS,
and cannot be send from another process through an API.

https://chromiumcodereview.appspot.com/895623002/diff/1/sdk/lib/io/process.da...
sdk/lib/io/process.dart:355: * Returns `true` if the signal is successfully sent
and process is killed.
On 2015/02/03 14:00:47, Lasse Reichstein Nielsen wrote:
> and process -> and the process
> 
> What if the process intercepts the SIGTERM signal? Does it return true in that
> case (I guess yes), but the process isn't killed.
> 
> SO:
>   Returns `true` if the signal is successfully sent.
> but skip" process is killed" because it might not be.

You are right. We can only see if the signal was send.

https://chromiumcodereview.appspot.com/895623002/diff/1/tests/standalone/io/p...
File tests/standalone/io/process_kill_test.dart (right):

https://chromiumcodereview.appspot.com/895623002/diff/1/tests/standalone/io/p...
tests/standalone/io/process_kill_test.dart:26: // Start a process that will hang
waiting for input until killed.
On 2015/02/03 14:00:47, Lasse Reichstein Nielsen wrote:
> You need to add startAsync/endAsync calls, otherwise the test succeedes before
> it has time to fail.
> Same goes for first test.

Thanks, done.

https://chromiumcodereview.appspot.com/895623002/diff/1/tests/standalone/io/p...
tests/standalone/io/process_kill_test.dart:32: Expect.isFalse(p.kill());
On 2015/02/03 14:00:47, Lasse Reichstein Nielsen wrote:
> Why not Process.killPid(p.pid) ?

Done.

[Søren Gjesse, 2015-02-04 08:20:43]

Committed patchset #2 (id:20001) manually as 43455 (presubmit successful).