[Ace-users] [ace-bugs] ACE_Process::spawn() results in undefined behavior

[Johnny Willemsen]

Hi JT,

Thanks for using the PRF form. I would prefer to use the !ACE_HAS_THREADS
instead of 0. Can you commit this to the repository?

Regards,

Johnny Willemsen
Remedy IT
Postbus 101
2650 AC  Berkel en Rodenrijs
The Netherlands
www.theaceorb.nl / www.remedy.nl  

*** Integrated compile and test statistics see
http://scoreboard.theaceorb.nl ***
*** Commercial service and support for ACE/TAO/CIAO             ***
*** See http://www.theaceorb.nl/en/support.html                 ***

>     ACE VERSION: 5.6.1 (subversion version 79840)
> 
>     HOST MACHINE and OPERATING SYSTEM:
> 
> NetBSD orac.acorntoolworks.com 3.0_STABLE NetBSD 3.0_STABLE 
> (ORAC) #0: Tue Feb 21 20:05:51 PST 2006  
> jtc at orac.acorntoolworks.com:/home/jtc/netbsd/NetBSD-3/src/sys/
> arch/i386/compile/ORAC i386
> 
>     TARGET MACHINE and OPERATING SYSTEM, if different from HOST:
> 
> NetBSD orac.acorntoolworks.com 3.0_STABLE NetBSD 3.0_STABLE 
> (ORAC) #0: Tue Feb 21 20:05:51 PST 2006  
> jtc at orac.acorntoolworks.com:/home/jtc/netbsd/NetBSD-3/src/sys/
> arch/i386/compile/ORAC i386
> 
>     COMPILER NAME AND VERSION (AND PATCHLEVEL):
> 
> gcc version 3.3.3 (NetBSD nb3 20040520)
> 
>     THE $ACE_ROOT/ace/config.h FILE [if you use a link to a platform-
>     specific file, simply state which one]:
> 
> config-netbsd.h
> 
>     THE $ACE_ROOT/include/makeinclude/platform_macros.GNU FILE [if you
>     use a link to a platform-specific file, simply state which one
>     (unless this isn't used in this case, e.g., with Microsoft Visual
>     C++)]:
> 
> platform_netbsd.GNU
> 
>     CONTENTS OF 
> $ACE_ROOT/bin/MakeProjectCreator/config/default.features
>     (used by MPC when you generate your own makefiles):
> 
> N/A
> 
>     AREA/CLASS/EXAMPLE AFFECTED:
>     [What example failed?  What module failed to compile?]
> 
> ACE_Process
> 
>     DOES THE PROBLEM AFFECT:
> 
> EXECUTION
> 
>     SYNOPSIS:
> 
> ACE_Process::spawn() results in undefined behavior 
> 
>     DESCRIPTION:
> 
> Similar to the PRF I sent this morning, ACE_Process::spawn() invokes
> async-signal-unsafe functions between fork() and exec().
> 
> This PRF is for the simplest of those cases.  In the child process,
> if setpgid(), setregid(), or setreuid() fails, it invokes ACE_ERROR
> to note the failure(s).  This will result in an async-signal-unsafe
> call to printf() at the very least (I wouldn't be surprised if tens
> of unsafe functions are called within a single ACE_ERROR invocation).
> 
> This can result in deadlock.
> 
>     REPEAT BY:
> 
> Discovered by code inspection.
> 
>     SAMPLE FIX/WORKAROUND:
> 
> The enclosed patch Adds a #if 0 around each ACE_ERROR call.  A case
> could be made for using #if !ACE_HAS_THREADS instead.  I'm interested
> in opinions over which would be preferable for the master ACE sources.
> 
> Index: Process.cpp
> ===================================================================
> --- Process.cpp	(revision 79869)
> +++ Process.cpp	(working copy)
> @@ -353,9 +353,15 @@
>        if (options.getgroup () != ACE_INVALID_PID
>            && ACE_OS::setpgid (0,
>                                options.getgroup ()) < 0)
> -        ACE_ERROR ((LM_ERROR,
> -                    ACE_TEXT ("%p.\n"),
> -                    ACE_TEXT ("ACE_Process::spawn: setpgid 
> failed.")));
> +	{
> +#if 0
> +	  // We can't emit this log message because ACE_ERROR(), etc.
> +	  // will invoke async signal unsafe functions.
> +	  ACE_ERROR ((LM_ERROR,
> +                      ACE_TEXT ("%p.\n"),
> +                      ACE_TEXT ("ACE_Process::spawn: setpgid 
> failed.")));
> +#endif
> +	}
>  # endif /* ACE_LACKS_SETPGID */
>  
>  # if !defined (ACE_LACKS_SETREGID)
> @@ -363,9 +369,15 @@
>            || options.getegid () != (uid_t) -1)
>          if (ACE_OS::setregid (options.getrgid (),
>                                options.getegid ()) == -1)
> -          ACE_ERROR ((LM_ERROR,
> -                      ACE_TEXT ("%p.\n"),
> -                      ACE_TEXT ("ACE_Process::spawn: 
> setregid failed.")));
> +	  {
> +#if 0
> +	    // We can't emit this log message because ACE_ERROR(), etc.
> +	    // will invoke async signal unsafe functions.
> +	    ACE_ERROR ((LM_ERROR,
> +			ACE_TEXT ("%p.\n"),
> +			ACE_TEXT ("ACE_Process::spawn: setregid 
> failed.")));
> +#endif
> +	  }
>  # endif /* ACE_LACKS_SETREGID */
>  
>  # if !defined (ACE_LACKS_SETREUID)
> @@ -374,9 +386,15 @@
>            || options.geteuid () != (uid_t) -1)
>          if (ACE_OS::setreuid (options.getruid (),
>                                options.geteuid ()) == -1)
> -          ACE_ERROR ((LM_ERROR,
> -                      ACE_TEXT ("%p.\n"),
> -                      ACE_TEXT ("ACE_Process::spawn: 
> setreuid failed.")));
> +	  {
> +#if 0
> +	    // We can't emit this log message because ACE_ERROR(), etc.
> +	    // will invoke async signal unsafe functions.
> +	    ACE_ERROR ((LM_ERROR,
> +			ACE_TEXT ("%p.\n"),
> +			ACE_TEXT ("ACE_Process::spawn: setreuid 
> failed.")));
> +#endif
> +	  }
>  # endif /* ACE_LACKS_SETREUID */
>  
>        this->child (ACE_OS::getppid ());
> 
> -- 
> J.T. Conklin
> 
> _______________________________________________
> ace-bugs mailing list
> ace-bugs at mail.cse.wustl.edu
> http://mail.cse.wustl.edu/mailman/listinfo/ace-bugs
>