Opened 11 years ago

Closed 4 years ago

#1944 closed task (no change required)

Kill command don't work with process names [Upstream to coreutils]

Reported by: tqh Owned by: phoudoin
Priority: normal Milestone: Unscheduled
Component: Applications/Command Line Tools Version: R1/Development
Keywords: Cc: Adek336, Prasad
Blocked By: Blocking: #4687, #8162
Has a Patch: yes Platform: All

Description

In BeOS R5 I always used

kill Tracker

when needed.

Unfortunatly Haiku's 'kill' doesn't support this fantastic feature.

Attachments (2)

0001-killall-add-killall-command.patch (11.7 KB) - added by Prasad 7 years ago.
0001-Add-support-to-kill-processes-using-their-names.patch (6.5 KB) - added by Prasad 7 years ago.

Download all attachments as: .zip

Change History (38)

comment:1 Changed 11 years ago by diver

Isn't killall is used for that?

comment:2 Changed 11 years ago by tqh

This bug is about kill. killall might do that, but it's not really related to the bug.

comment:3 Changed 10 years ago by Adek336

killall doesn't seem to be included with Haiku

~> killall
sh: killall: command not found

it'd be cool to have it, though.

comment:4 Changed 10 years ago by Adek336

Cc: adek336@… added

comment:5 Changed 8 years ago by stpere

In R5, if you have several instances of a process named, let say "test", did kill test terminate them all or the first it found, or?

comment:6 Changed 7 years ago by diver

Blocking: 8162 added

comment:7 Changed 7 years ago by diver

Blocking: 4687 added

comment:8 Changed 7 years ago by diver

Component: ApplicationsApplications/Command Line Tools

Changed 7 years ago by Prasad

comment:9 Changed 7 years ago by Prasad

Has a Patch: set

comment:10 Changed 7 years ago by Prasad

Cc: prasadjoshi124@… added

comment:11 Changed 7 years ago by diver

It would be nice to have this functionality in kill without adding one more tool.

comment:12 in reply to:  11 ; Changed 7 years ago by kallisti5

Replying to diver:

It would be nice to have this functionality in kill without adding one more tool.

Yeah, but it really isn't the posix way. killall is more correct.

comment:13 in reply to:  12 ; Changed 7 years ago by anevilyak

Replying to kallisti5:

Yeah, but it really isn't the posix way. killall is more correct.

That may be the case, but we're not always aiming to limit ourselves to exactly what the posix spec says. In this case, I'd say it's more convenient to the user to also support it in kill, which I might add R5 did as well.

comment:14 in reply to:  13 ; Changed 7 years ago by kallisti5

Replying to anevilyak:

Replying to kallisti5:

Yeah, but it really isn't the posix way. killall is more correct.

That may be the case, but we're not always aiming to limit ourselves to exactly what the posix spec says. In this case, I'd say it's more convenient to the user to also support it in kill, which I might add R5 did as well.

Ah.. if R5 did it then yeah, kill may be the better place for this :)

comment:15 in reply to:  14 Changed 7 years ago by Prasad

Replying to kallisti5:

Replying to anevilyak:

Replying to kallisti5:

Yeah, but it really isn't the posix way. killall is more correct.

That may be the case, but we're not always aiming to limit ourselves to exactly what the posix spec says. In this case, I'd say it's more convenient to the user to also support it in kill, which I might add R5 did as well.

Ah.. if R5 did it then yeah, kill may be the better place for this :)

killall has lots of switches for example killing group, regex search, checking the process running specific path. If kill command cannot support all of them, then I would suggest we do both.

I am not sure all the options supported by kill on R5. It would be great if anyone can print the usage message for kill command.

comment:16 Changed 7 years ago by diver

$ kill
Usage : kill [-SIGNAME] pid1 [pid2 ...]
        where SIGNAME is one of INT, HUP, KILL, etc

comment:17 in reply to:  16 Changed 7 years ago by Prasad

Replying to diver:

$ kill
Usage : kill [-SIGNAME] pid1 [pid2 ...]
        where SIGNAME is one of INT, HUP, KILL, etc

Seems like R5 too does not support kill <name>. There is no such option.

comment:18 Changed 7 years ago by diver

Nah, R5 does support this feature even though it's not exposed in its' usage text.
In other words kill Tracker worked fine.

comment:19 Changed 7 years ago by leavengood

I think adding name support to kill is fine, but in general for this sort of thing "hey Tracker QUIT" is nicer, at least for BApplications.

comment:20 Changed 7 years ago by diver

Well hey can't be used to kill processes.

comment:21 Changed 7 years ago by diver

Cc: Adek336 Prasad added; adek336@… prasadjoshi124@… removed
Version: R1/pre-alpha1R1/Development

comment:22 in reply to:  18 Changed 7 years ago by Prasad

Replying to diver:

Nah, R5 does support this feature even though it's not exposed in its' usage text.
In other words kill Tracker worked fine.

Haiku uses kill command which is by default built into the shell (not /bin/kill). The code in this command does not access the system wide list of processes. It works mostly with the list of processes ran on the current shell. One one exception to this is when kill is invoked with a pid, however it directly uses the PID and does no search.

We can add a special case for kill <name> and write the code to look for all the processes running in the system. However, that defeats the purpose of using shell builtin command.

After a small modification in the code, it was possible to kill the current shell jobs using process name. However, it did not kill processes ran by on other shell. For example:

~> jobs                     # NOTE NO JOBS RUNNING IN BACKGROUND

~> ps
Team                                                  Id #Threads  Gid  Uid
kernel_team                                            1       32    0    0
/boot/system/servers/registrar                        48        6    0    0
/boot/system/servers/debug_server                     55        2    0    0
/boot/system/servers/net_server                       56        4    0    0
/boot/system/servers/app_server                       57       26    0    0
/boot/system/servers/syslog_daemon                    72        2    0    0
/boot/system/servers/input_server                     84        9    0    0
/boot/system/servers/mount_server                     94        1    0    0
/boot/system/Tracker                                 106        7    0    0
/boot/system/Deskbar                                 107        2    0    0
/boot/system/servers/media_server                    108        6    0    0
/boot/system/servers/midi_server                     109        3    0    0
/boot/system/servers/print_server                    110        2    0    0
/boot/system/servers/cddb_daemon                     112        1    0    0
/boot/system/servers/notification_server             113        2    0    0
/boot/system/servers/power_daemon                    114        1    0    0
/boot/system/servers/media_addon_server              152        5    0    0
/boot/system/apps/LaunchBox                          217        2    0    0
/boot/system/apps/Terminal                           242        6    0    0
/bin/bash -l                                         246        1    0    0
/bin/bash -l                                         263        1    0    0
/bin/sleep 10000                                     278        1    0    0
/bin/ps                                              279        1    0    0

~> kill -9 sleep
bash: kill: sleep: no such job

~> sleep 1000 &           # RAN A PROCESS IN BACKGROUND
[1] 281

~> ps
Team                                                  Id #Threads  Gid  Uid
kernel_team                                            1       32    0    0
/boot/system/servers/registrar                        48        6    0    0
/boot/system/servers/debug_server                     55        2    0    0
/boot/system/servers/net_server                       56        4    0    0
/boot/system/servers/app_server                       57       26    0    0
/boot/system/servers/syslog_daemon                    72        2    0    0
/boot/system/servers/input_server                     84        9    0    0
/boot/system/servers/mount_server                     94        1    0    0
/boot/system/Tracker                                 106        7    0    0
/boot/system/Deskbar                                 107        2    0    0
/boot/system/servers/media_server                    108        6    0    0
/boot/system/servers/midi_server                     109        3    0    0
/boot/system/servers/print_server                    110        2    0    0
/boot/system/servers/cddb_daemon                     112        1    0    0
/boot/system/servers/notification_server             113        2    0    0
/boot/system/servers/power_daemon                    114        1    0    0
/boot/system/servers/media_addon_server              152        5    0    0
/boot/system/apps/LaunchBox                          217        2    0    0
/boot/system/apps/Terminal                           242        6    0    0
/bin/bash -l                                         246        1    0    0
/bin/bash -l                                         263        1    0    0
/bin/sleep 10000                                     278        1    0    0
/bin/sleep 1000                                      281        1    0    0
/bin/ps                                              282        1    0    0


~> jobs                            # ONE JOB RUNNING
[1]+  Running                 sleep 1000 &

~> kill -9 sleep
killing this process.

~> jobs
[1]+  Kill Thread             sleep 1000

~> ps
Team                                                  Id #Threads  Gid  Uid
kernel_team                                            1       32    0    0
/boot/system/servers/registrar                        48        6    0    0
/boot/system/servers/debug_server                     55        2    0    0
/boot/system/servers/net_server                       56        4    0    0
/boot/system/servers/app_server                       57       26    0    0
/boot/system/servers/syslog_daemon                    72        2    0    0
/boot/system/servers/input_server                     84        9    0    0
/boot/system/servers/mount_server                     94        1    0    0
/boot/system/Tracker                                 106        7    0    0
/boot/system/Deskbar                                 107        2    0    0
/boot/system/servers/media_server                    108        6    0    0
/boot/system/servers/midi_server                     109        3    0    0
/boot/system/servers/print_server                    110        2    0    0
/boot/system/servers/cddb_daemon                     112        1    0    0
/boot/system/servers/notification_server             113        2    0    0
/boot/system/servers/power_daemon                    114        1    0    0
/boot/system/servers/media_addon_server              152        5    0    0
/boot/system/apps/LaunchBox                          217        2    0    0
/boot/system/apps/Terminal                           242        6    0    0
/bin/bash -l                                         246        1    0    0
/bin/bash -l                                         263        1    0    0
/bin/sleep 10000                                     278        1    0    0
/bin/ps                                              283        1    0    0

To summarize


  1. Changes to support name killing are possible in builtin, however they are not recommended (according to me).
  1. We can do necessary modification in the non-builtin kill command. However, the command must be invoked using full path of the command /bin/kill
  1. Change /bin/kill and remove bulitin kill.

comment:23 Changed 7 years ago by diver

BeOS R5:

$ type kill
kill is hashed (/bin/kill)

Haiku:

~> type kill
kill is a shell builtin
Last edited 7 years ago by diver (previous) (diff)

comment:24 Changed 7 years ago by phoudoin

The simple fact that there is not /bin/kill under Haiku is already a potential binary compatibility issue, if a BeOS app was running it outside a shell context (read exec() vs system() for example). Don't expect me explain why one would want to do that, that's not the point here...

So I'm for moving to genuine /bin/kill command tool and, obviously, add process name support in the same... well, process ;-)

Last edited 7 years ago by phoudoin (previous) (diff)

comment:25 Changed 6 years ago by diver

Could anyone review this patch please?

comment:26 Changed 6 years ago by leavengood

Owner: changed from axeld to leavengood
Status: newassigned

+1 to this patch, with the exception that some of the variable names in the new functions in kill are too short (e and n in is_number(), and tm, tc, p and q in kill_by_name().) In addition the "int l" in kill_by_name is not used, as far as I can see.

Overall, very nice patch Prasad! If Prasad is around and willing to fix the variable names, please do. Otherwise I'll apply this patch and fix the variable names myself before the end of the week.

comment:27 Changed 6 years ago by Giova84

Any news on this ticket? Would be a very useful feature the ability to kill a process by its name! :-)

comment:28 Changed 6 years ago by phoudoin

Well, I guess leavengood's week is not ended yet ;-) If he don't beat me, I'll fix and apply it.

comment:29 Changed 6 years ago by phoudoin

Owner: changed from leavengood to phoudoin
Status: assignedin-progress

comment:30 Changed 6 years ago by phoudoin

Resolution: fixed
Status: in-progressclosed

Applied with some style and code cleanup in hrev45523. Thanks.

comment:31 Changed 6 years ago by Giova84

Hey Phoudin, thanks to you for this little but very useful improvement! :-)

Version 0, edited 6 years ago by Giova84 (next)

comment:32 Changed 6 years ago by phoudoin

Type: bugenhancement

Re-opened to not forget this should make this upstream compliant, as suggested by mmu_man.

comment:33 Changed 6 years ago by phoudoin

Resolution: fixed
Status: closedreopened

comment:34 in reply to:  31 Changed 6 years ago by phoudoin

Replying to Giova84:

Hey Phoudoin, thanks to you for this little but very useful improvement! :-)

What's 5 years waiting for such improvement? ;-)

comment:35 Changed 4 years ago by pulkomandy

Milestone: R1Unscheduled
Summary: Kill command don't work with process namesKill command don't work with process names [Upstream to coreutils]
Type: enhancementtask

comment:36 Changed 4 years ago by waddlesplash

Resolution: no change required
Status: reopenedclosed

The "kill" command now uses the Bash builtin. Also, GNU would not accept our patch anyway, as they have another utility ("killall") that does this.

If you're experiencing some other problem with the patched Bash kill, please file a ticket at HaikuPorts.

Note: See TracTickets for help on using tickets.