)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":18893,"name":"Miles Gould","email":"mgould@redhat.com","username":"pozorvlak"},"change_message_id":"f8e7d582063ad4e61561d5d8b2909468044680e0","unresolved":false,"context_lines":[{"line_number":9,"context_line":"functions-common:screen_it utilizes _old_run_process in case of a"},{"line_number":10,"context_line":"non-screen environment such as grenade testing.  However this call-chain"},{"line_number":11,"context_line":"fails to store correct PID/PGID in order for a service to be stopped by"},{"line_number":12,"context_line":"screen_stop later on.  The issue boils down to following (simplified) code:"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"    function _old_run_process() {"},{"line_number":15,"context_line":"        local command\u003d$2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3aaa91ec_cf404ba8","line":12,"updated":"2016-06-23 14:20:11.000000000","message":"Nit: \"the following\"","commit_id":"33e05294258ab6e60488f31eb84967e25283585a"},{"author":{"_account_id":18893,"name":"Miles Gould","email":"mgould@redhat.com","username":"pozorvlak"},"change_message_id":"f8e7d582063ad4e61561d5d8b2909468044680e0","unresolved":false,"context_lines":[{"line_number":24,"context_line":"the service \"$command\" pid.  The screen_stop chain uses pkill -g (-F)"},{"line_number":25,"context_line":"state/service.pid to terminate the whole service process group."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Since pid of the subshell is saved instead of the pgid of the service,"},{"line_number":28,"context_line":"pkill -g fails with exit code 1 (target not found).  If a correct pgid was"},{"line_number":29,"context_line":"saved instead (echo `ps -opgid\u003d $!` \u003e state/service.pid), due to pgid"},{"line_number":30,"context_line":"implicit inheritance the pkill command would have killed the whole grenade"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3aaa91ec_2451310f","line":27,"updated":"2016-06-23 14:20:11.000000000","message":"Nit: \"the pid\"","commit_id":"33e05294258ab6e60488f31eb84967e25283585a"},{"author":{"_account_id":18893,"name":"Miles Gould","email":"mgould@redhat.com","username":"pozorvlak"},"change_message_id":"f8e7d582063ad4e61561d5d8b2909468044680e0","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Since pid of the subshell is saved instead of the pgid of the service,"},{"line_number":28,"context_line":"pkill -g fails with exit code 1 (target not found).  If a correct pgid was"},{"line_number":29,"context_line":"saved instead (echo `ps -opgid\u003d $!` \u003e state/service.pid), due to pgid"},{"line_number":30,"context_line":"implicit inheritance the pkill command would have killed the whole grenade"},{"line_number":31,"context_line":"process group."},{"line_number":32,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3aaa91ec_553fc5d5","line":29,"updated":"2016-06-23 14:20:11.000000000","message":"You don\u0027t need the backticks: ps -opgid\u003d $! \u003e state/service.pid","commit_id":"33e05294258ab6e60488f31eb84967e25283585a"},{"author":{"_account_id":18653,"name":"Milan Kováčik","email":"mkovacik@redhat.com","username":"dparalen"},"change_message_id":"da106bb35b266bd3e73895f91dafdd573d2d4a04","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Since pid of the subshell is saved instead of the pgid of the service,"},{"line_number":28,"context_line":"pkill -g fails with exit code 1 (target not found).  If a correct pgid was"},{"line_number":29,"context_line":"saved instead (echo `ps -opgid\u003d $!` \u003e state/service.pid), due to pgid"},{"line_number":30,"context_line":"implicit inheritance the pkill command would have killed the whole grenade"},{"line_number":31,"context_line":"process group."},{"line_number":32,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3aaa91ec_cb6af1b2","line":29,"in_reply_to":"3aaa91ec_553fc5d5","updated":"2016-06-23 15:35:43.000000000","message":"correct but it\u0027s not part of the fix.","commit_id":"33e05294258ab6e60488f31eb84967e25283585a"},{"author":{"_account_id":18893,"name":"Miles Gould","email":"mgould@redhat.com","username":"pozorvlak"},"change_message_id":"f8e7d582063ad4e61561d5d8b2909468044680e0","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    echo $! \u003e state/service.pid"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Because exec is used here, setsid isn\u0027t forked as a separate process but"},{"line_number":46,"context_line":"rather replaces subshell process instance while maintaining all"},{"line_number":47,"context_line":"redirection.  Setsid uses its pid to create new pgid and sid of a"},{"line_number":48,"context_line":"\"$command\" that ist\u0027s going to exec, again, without forking.  Since that"},{"line_number":49,"context_line":"pid is stored in the parent shell ($!), it can be used as the pgid (and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3aaa91ec_500d1336","line":46,"updated":"2016-06-23 14:20:11.000000000","message":"nit: \"the subshell process instance\"","commit_id":"33e05294258ab6e60488f31eb84967e25283585a"},{"author":{"_account_id":18893,"name":"Miles Gould","email":"mgould@redhat.com","username":"pozorvlak"},"change_message_id":"f8e7d582063ad4e61561d5d8b2909468044680e0","unresolved":false,"context_lines":[{"line_number":45,"context_line":"Because exec is used here, setsid isn\u0027t forked as a separate process but"},{"line_number":46,"context_line":"rather replaces subshell process instance while maintaining all"},{"line_number":47,"context_line":"redirection.  Setsid uses its pid to create new pgid and sid of a"},{"line_number":48,"context_line":"\"$command\" that ist\u0027s going to exec, again, without forking.  Since that"},{"line_number":49,"context_line":"pid is stored in the parent shell ($!), it can be used as the pgid (and"},{"line_number":50,"context_line":"sid) of the service process eventually forked by bash in the \"$command\"."},{"line_number":51,"context_line":"This pgid being different from parent shell\u0027s, it is safe to pkill -g that"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3aaa91ec_75580133","line":48,"updated":"2016-06-23 14:20:11.000000000","message":"s/ist/it","commit_id":"33e05294258ab6e60488f31eb84967e25283585a"},{"author":{"_account_id":18893,"name":"Miles Gould","email":"mgould@redhat.com","username":"pozorvlak"},"change_message_id":"f8e7d582063ad4e61561d5d8b2909468044680e0","unresolved":false,"context_lines":[{"line_number":47,"context_line":"redirection.  Setsid uses its pid to create new pgid and sid of a"},{"line_number":48,"context_line":"\"$command\" that ist\u0027s going to exec, again, without forking.  Since that"},{"line_number":49,"context_line":"pid is stored in the parent shell ($!), it can be used as the pgid (and"},{"line_number":50,"context_line":"sid) of the service process eventually forked by bash in the \"$command\"."},{"line_number":51,"context_line":"This pgid being different from parent shell\u0027s, it is safe to pkill -g that"},{"line_number":52,"context_line":"pgid to stop the service only (the forked service has ppid\u003d\u003d1,"},{"line_number":53,"context_line":"pgid\u003d\u003dsid\u003d\u003d$!, pid\u003d\u003d\u003cnew pid\u003e). That effectively turns the service into a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3aaa91ec_46139d0d","line":50,"updated":"2016-06-23 14:20:11.000000000","message":"So the end result is that the pid in $! gets passed through several layers of execution, rather than being replaced with a new and unknown pgid?","commit_id":"33e05294258ab6e60488f31eb84967e25283585a"},{"author":{"_account_id":18653,"name":"Milan Kováčik","email":"mkovacik@redhat.com","username":"dparalen"},"change_message_id":"da106bb35b266bd3e73895f91dafdd573d2d4a04","unresolved":false,"context_lines":[{"line_number":47,"context_line":"redirection.  Setsid uses its pid to create new pgid and sid of a"},{"line_number":48,"context_line":"\"$command\" that ist\u0027s going to exec, again, without forking.  Since that"},{"line_number":49,"context_line":"pid is stored in the parent shell ($!), it can be used as the pgid (and"},{"line_number":50,"context_line":"sid) of the service process eventually forked by bash in the \"$command\"."},{"line_number":51,"context_line":"This pgid being different from parent shell\u0027s, it is safe to pkill -g that"},{"line_number":52,"context_line":"pgid to stop the service only (the forked service has ppid\u003d\u003d1,"},{"line_number":53,"context_line":"pgid\u003d\u003dsid\u003d\u003d$!, pid\u003d\u003d\u003cnew pid\u003e). That effectively turns the service into a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3aaa91ec_a26cea35","line":50,"in_reply_to":"3aaa91ec_46139d0d","updated":"2016-06-23 15:35:43.000000000","message":"The way to get a pgid different from one\u0027s shell is the reason for the indirection. Otherwise the pkill -g $command pgid would kill the grenade.sh too.\n\n{ exec setsid /bin/bash -c $command; } \u0026 can be read as:\nfork(); subshell-exec(setsid-tool); setsid-tool.fork(); setsid-tool-child.libc.setsid() ; setsid-tool-child.exec(bash -c $command)\nThe reason for { exec setsid ... } \u0026 is not to lose the pgid created by setsid; this way bash -c $command and $command will have same pgid as $! (because the $!-subshell is replaced by the setsid tool through the exec statement and because pgids are inherited)\n\nStrictly speaking, one might want to ps -o pgid\u003d $! \u003e service.pid rather than echo $! \u003e service.pid\nI kept echo $! \u003e service.pid there as $! \u003d\u003d $(ps -o pgid\u003d $!) \u003d\u003d $(ps -o pgid\u003d \"$command\") in this case.\n\nI didn\u0027t want the change to be bigger than necessary.\nMaybe I shall update the fix to explicitly get the pgid of $! instead but then the service.pid is a misleading file name","commit_id":"33e05294258ab6e60488f31eb84967e25283585a"},{"author":{"_account_id":18893,"name":"Miles Gould","email":"mgould@redhat.com","username":"pozorvlak"},"change_message_id":"f706ba2a9d428e31d96cf300a8ce25c0cfb78bc4","unresolved":false,"context_lines":[{"line_number":47,"context_line":"redirection.  Setsid uses its pid to create new pgid and sid of a"},{"line_number":48,"context_line":"\"$command\" that ist\u0027s going to exec, again, without forking.  Since that"},{"line_number":49,"context_line":"pid is stored in the parent shell ($!), it can be used as the pgid (and"},{"line_number":50,"context_line":"sid) of the service process eventually forked by bash in the \"$command\"."},{"line_number":51,"context_line":"This pgid being different from parent shell\u0027s, it is safe to pkill -g that"},{"line_number":52,"context_line":"pgid to stop the service only (the forked service has ppid\u003d\u003d1,"},{"line_number":53,"context_line":"pgid\u003d\u003dsid\u003d\u003d$!, pid\u003d\u003d\u003cnew pid\u003e). That effectively turns the service into a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3aaa91ec_a607d716","line":50,"in_reply_to":"3aaa91ec_a26cea35","updated":"2016-06-23 16:22:15.000000000","message":"Sorry, this is a bit beyond my Unix knowledge :-( Is the following correct?\n\n1. The aim is to have the command and all its children (and only those processes) using a pgid which we know.\n\n2. We need setsid to create a new pgid.\n\n3. The \"\u0026\" syntax forks off a subshell and stores its pid in $!.\n\n4. By using \"exec setsid\" we (a) create a new process group, (b) ensure that its pgid is equal to the process calling \"exec\".\n\n5. So { exec setsid bash -c $command } \u0026 ensures that \"bash -c $command\" and its descendants form a process group, and that their pgid is stored in $!, achieving 1.","commit_id":"33e05294258ab6e60488f31eb84967e25283585a"}]}