[PATCH v2] prserv/serv: Tweak stdout manipulation to be stream safe

[PATCH v2] prserv/serv: Tweak stdout manipulation to be stream safe

[Richard Purdie]

We've been seeing oe-selftest failures under puzzling circumstances. It
turns out if you run oe-selftest on a machine with xmlrunner installed
and have the recent tinfoil2 changes, the launching of PR server crashes
leading to selftest hanging if using an autoloaded PR server.

The reason is that xmlrunner uses an io.StringIO object as stdout/stderr
instead of the usual io.TextIOWrapper and StringIO lacks a fileno() method.

We have to deal with both cases and in the python way, we try and then seek
forgivness if we see an AttributeError or UnSupportedOperation exception.
Unfortunately we have to deal with both cases as we may be performing a
traditiional double fork() from the commandline, or a larger python program.

[YOCTO #10866]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/prserv/serv.py | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 350b085..d9b602f 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -242,12 +242,25 @@ class PRServer(SimpleXMLRPCServer):
 
         sys.stdout.flush()
         sys.stderr.flush()
+
+        # We could be called from a python thread with io.StringIO as
+        # stdout/stderr or it could be 'real' unix fd forking where we need
+        # to physically close the fds to prevent the program launching us from
+        # potentially hanging on a pipe. Handle both cases.
         si = open('/dev/null', 'r')
+        try:
+            os.dup2(si.fileno(),sys.stdin.fileno())
+        except (AttributeError, io.UnsupportedOperation):
+            sys.stdin = si
         so = open(self.logfile, 'a+')
-        se = so
-        os.dup2(si.fileno(),sys.stdin.fileno())
-        os.dup2(so.fileno(),sys.stdout.fileno())
-        os.dup2(se.fileno(),sys.stderr.fileno())
+        try:
+            os.dup2(so.fileno(),sys.stdout.fileno())
+        except (AttributeError, io.UnsupportedOperation):
+            sys.stdout = so
+        try:
+            os.dup2(so.fileno(),sys.stderr.fileno())
+        except (AttributeError, io.UnsupportedOperation):
+            sys.stderr = so
 
         # Clear out all log handlers prior to the fork() to avoid calling
         # event handlers not part of the PRserver
-- 
2.7.4

Re: [PATCH v2] prserv/serv: Tweak stdout manipulation to be stream safe

[Joshua Lock]

On Thu, 2017-01-05 at 13:52 +0000, Richard Purdie wrote:
> We've been seeing oe-selftest failures under puzzling circumstances.
> It
> turns out if you run oe-selftest on a machine with xmlrunner
> installed
> and have the recent tinfoil2 changes, the launching of PR server
> crashes
> leading to selftest hanging if using an autoloaded PR server.
> 
> The reason is that xmlrunner uses an io.StringIO object as
> stdout/stderr
> instead of the usual io.TextIOWrapper and StringIO lacks a fileno()
> method.
> 
> We have to deal with both cases and in the python way, we try and
> then seek
> forgivness if we see an AttributeError or UnSupportedOperation
> exception.
> Unfortunately we have to deal with both cases as we may be performing
> a
> traditiional double fork() from the commandline, or a larger python
> program.
> 
> [YOCTO #10866]
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  lib/prserv/serv.py | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
> index 350b085..d9b602f 100644
> --- a/lib/prserv/serv.py
> +++ b/lib/prserv/serv.py
> @@ -242,12 +242,25 @@ class PRServer(SimpleXMLRPCServer):
>  
>          sys.stdout.flush()
>          sys.stderr.flush()
> +
> +        # We could be called from a python thread with io.StringIO
> as
> +        # stdout/stderr or it could be 'real' unix fd forking where
> we need
> +        # to physically close the fds to prevent the program
> launching us from
> +        # potentially hanging on a pipe. Handle both cases.
>          si = open('/dev/null', 'r')
> +        try:
> +            os.dup2(si.fileno(),sys.stdin.fileno())
> +        except (AttributeError, io.UnsupportedOperation):
> +            sys.stdin = si
>          so = open(self.logfile, 'a+')
> -        se = so
> -        os.dup2(si.fileno(),sys.stdin.fileno())
> -        os.dup2(so.fileno(),sys.stdout.fileno())
> -        os.dup2(se.fileno(),sys.stderr.fileno())
> +        try:
> +            os.dup2(so.fileno(),sys.stdout.fileno())
> +        except (AttributeError, io.UnsupportedOperation):
> +            sys.stdout = so
> +        try:
> +            os.dup2(so.fileno(),sys.stderr.fileno())
> +        except (AttributeError, io.UnsupportedOperation):
> +            sys.stderr = so

I think there's a copy/paste error here — shouldn't the second
try/except be duplicating/assigning se not so?

Joshua

>  
>          # Clear out all log handlers prior to the fork() to avoid
> calling
>          # event handlers not part of the PRserver
> -- 
> 2.7.4
> 

Re: [PATCH v2] prserv/serv: Tweak stdout manipulation to be stream safe

[Richard Purdie]

On Thu, 2017-01-05 at 14:46 +0000, Joshua Lock wrote:
> On Thu, 2017-01-05 at 13:52 +0000, Richard Purdie wrote:
> > +        # to physically close the fds to prevent the program
> > launching us from
> > +        # potentially hanging on a pipe. Handle both cases.
> >          si = open('/dev/null', 'r')
> > +        try:
> > +            os.dup2(si.fileno(),sys.stdin.fileno())
> > +        except (AttributeError, io.UnsupportedOperation):
> > +            sys.stdin = si
> >          so = open(self.logfile, 'a+')
> > -        se = so
> > -        os.dup2(si.fileno(),sys.stdin.fileno())
> > -        os.dup2(so.fileno(),sys.stdout.fileno())
> > -        os.dup2(se.fileno(),sys.stderr.fileno())
> > +        try:
> > +            os.dup2(so.fileno(),sys.stdout.fileno())
> > +        except (AttributeError, io.UnsupportedOperation):
> > +            sys.stdout = so
> > +        try:
> > +            os.dup2(so.fileno(),sys.stderr.fileno())
> > +        except (AttributeError, io.UnsupportedOperation):
> > +            sys.stderr = so
> I think there's a copy/paste error here — shouldn't the second
> try/except be duplicating/assigning se not so?

so == se in the original code and I dropped that just using so
everywhere here so I believe the code is ok.

Cheers,

Richard