Re: oaf patch to keep servers in process group

[Maciej Stachowiak <mjs noisehavoc org>]

On 20Jul2001 07:34PM (-0400), Havoc Pennington wrote:
> 
> Hi,
> 
> Attached patch keeps servers spawned by oafd in the oafd process
> group.
> 
> Basically what this gives you is a slightly-less-broken equivalent of
> oaf-slay, "killall -g oafd"

That's fine. Is your code portable to both BSD and System V systems? I
know from reading Advanced Programming in the Unix Environment that
they have slightly different semantics about process groups and
sessions. It would be nice actually if oafd would properly daemonize
itself in the way recommended by APUE.

> Discussed the real fix a bit more with Elliot, but we didn't really
> figure it out yet. There's serious confusion about the scope of oafd
> (session, display, machine, home directory, domain) and its lifecycle,
> and also the scope/lifecycle of the servers it spawns.  There needs to
> be some rational solution to this sometime which allows users to both
> display X apps on a remote display, and also have two displays on the
> same machine.

Right now, oafd is per-user per-machine. My plan to deal with the life
cycle problems is to make oafd exit after some period of inactivity
and transparently restart. I don't know how soon I will get to this though.
 

> 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/oaf/ChangeLog,v
> retrieving revision 1.126.2.66
> diff -u -r1.126.2.66 ChangeLog
> --- ChangeLog	2001/07/20 00:10:53	1.126.2.66
> +++ ChangeLog	2001/07/20 23:21:28
> @@ -1,3 +1,16 @@
> +2001-07-20  Havoc Pennington  <hp redhat com>
> +
> +	* oafd/main.c (main): become a process group leader
> +	on startup
> +
> +	* oafd/od-activate.c (od_server_activate_exe): use 
> +	oaf_internal_server_by_forking_extended() to set process group
> +	of activated servers
> +
> +	* liboaf/oaf-fork-server.c
> +	(oaf_internal_server_by_forking_extended): 
> +	new function to allow setting process group of the child
> +
>  2001-07-20  Richard Hult  <rhult codefactory se>
>  
>  	* liboaf/oaf-activate.c (oaf_activate_from_id): Move the code that
> Index: liboaf/liboaf-private.h
> ===================================================================
> RCS file: /cvs/gnome/oaf/liboaf/liboaf-private.h,v
> retrieving revision 1.12.2.2
> diff -u -r1.12.2.2 liboaf-private.h
> --- liboaf/liboaf-private.h	2001/07/16 18:21:13	1.12.2.2
> +++ liboaf/liboaf-private.h	2001/07/20 23:21:28
> @@ -60,13 +60,19 @@
>  
>  #define OAF_STR_NULL_OR_MATCH(x, y) ((x == NULL) || (x != NULL && y != NULL && strcmp (x, y) != 0))
>  
> -/* Rename oaf_service_get () in unstable branch */
>  CORBA_Object oaf_internal_service_get_extended  (const OAFBaseService         *base_service,
>                                                   gboolean                     existing_only,
>                                                   CORBA_Environment          *ev);
>  CORBA_Object oaf_internal_activation_context_get_extended (gboolean           existing_only,
>                                                             CORBA_Environment *ev);
>  
> +
> +CORBA_Object oaf_internal_server_by_forking_extended (const char **cmd,
> +                                                      gboolean set_process_group,
> +                                                      int fd_arg, 
> +                                                      const char *display,
> +                                                      const char *od_iorstr,
> +                                                      CORBA_Environment * ev);
>  
>  #endif
>  
> Index: liboaf/oaf-fork-server.c
> ===================================================================
> RCS file: /cvs/gnome/oaf/liboaf/oaf-fork-server.c,v
> retrieving revision 1.1.2.2
> diff -u -r1.1.2.2 oaf-fork-server.c
> --- liboaf/oaf-fork-server.c	2001/07/16 18:21:13	1.1.2.2
> +++ liboaf/oaf-fork-server.c	2001/07/20 23:21:28
> @@ -131,6 +131,19 @@
>  		       const char *od_iorstr,
>                         CORBA_Environment * ev)
>  {
> +        return oaf_internal_server_by_forking_extended (cmd, FALSE, fd_arg,
> +                                                        display, od_iorstr,
> +                                                        ev);
> +}
> +
> +CORBA_Object
> +oaf_internal_server_by_forking_extended (const char **cmd,
> +                                         gboolean set_process_group,
> +                                         int fd_arg, 
> +                                         const char *display,
> +                                         const char *od_iorstr,
> +                                         CORBA_Environment * ev)
> +{
>  	gint iopipes[2];
>  	CORBA_Object retval = CORBA_OBJECT_NIL;
>  	OAF_GeneralError *errval;
> @@ -142,7 +155,8 @@
>          guint watchid;
>          struct sigaction sa;
>          sigset_t mask, omask;
> -                
> +        int parent_pid;
> +        
>       	pipe (iopipes);
>  
>          /* Block SIGCHLD so no one else can wait() on the child before us. */
> @@ -150,6 +164,8 @@
>          sigaddset (&mask, SIGCHLD);
>          sigprocmask (SIG_BLOCK, &mask, &omask);
>  
> +        parent_pid = getpid ();
> +        
>  	/* fork & get the IOR from the magic pipe */
>  	childpid = fork ();
>  
> @@ -258,11 +274,20 @@
>                          cmd[fd_arg] = g_strdup_printf (cmd[fd_arg], iopipes[1]);
>                  }
>  
> -		setsid ();
>  		memset (&sa, 0, sizeof (sa));
>  		sa.sa_handler = SIG_IGN;
>  		sigaction (SIGPIPE, &sa, 0);
>  
> +                if (set_process_group) {
> +                        if (setpgid (getpid (), parent_pid) < 0) {
> +                                g_print (_("OAF failed to set process group of %s: %s\n"),
> +                                         cmd[0], g_strerror (errno));
> +                                _exit (1);
> +                        }
> +                } else {
> +                        setsid ();
> +                }
> +                
>  		execvp (cmd[0], (char **) cmd);
>  		if (iopipes[1] != 1)
>  			dup2 (iopipes[1], 1);
> Index: oafd/main.c
> ===================================================================
> RCS file: /cvs/gnome/oaf/oafd/main.c,v
> retrieving revision 1.22.4.6
> diff -u -r1.22.4.6 main.c
> --- oafd/main.c	2001/06/15 01:50:13	1.22.4.6
> +++ oafd/main.c	2001/07/20 23:21:28
> @@ -102,6 +102,11 @@
>  		exit (EXIT_FAILURE);
>  	}
>  
> +        /* Become process group leader, detach from controlling terminal,
> +         * etc.
> +         */
> +        setsid ();
> +        
>          /* This is needed because otherwise, if oafd persists across X
>           * sessions, spawned processes will inherit an invalid value of
>           * SESSION_MANAGER and be very very slow while attempting to 
> Index: oafd/od-activate.c
> ===================================================================
> RCS file: /cvs/gnome/oaf/oafd/od-activate.c,v
> retrieving revision 1.15.2.1
> diff -u -r1.15.2.1 od-activate.c
> --- oafd/od-activate.c	2001/03/01 10:10:48	1.15.2.1
> +++ oafd/od-activate.c	2001/07/20 23:21:28
> @@ -173,9 +173,15 @@
>  	args[i] = NULL;
>  
>          display = oafd_CORBA_Context_get_value (actinfo->ctx, "display", NULL, ev);
> -
> -	retval = oaf_server_by_forking ((const char **) args, fd_arg, display,
> -					iorstr, ev);
> +        
> +        /* We set the process group of activated servers to our process group;
> +         * this allows people to destroy all OAF servers along with oafd
> +         * if necessary
> +         */
> +	retval = oaf_internal_server_by_forking_extended ((const char **) args,
> +                                                          TRUE,
> +                                                          fd_arg, display,
> +                                                          iorstr, ev);
>          
>          g_free (display);
>  	CORBA_free (iorstr);