[PATCH] python/semanage: empty stdout before exiting on BrokenPipeError

[PATCH] python/semanage: empty stdout before exiting on BrokenPipeError

[Vit Mojzis]

Empty stdout buffer before exiting when BrokenPipeError is
encountered. Otherwise python will flush the bufer during exit, which
may trigger the exception again.
https://docs.python.org/3/library/signal.html#note-on-sigpipe

Fixes:
   #semanage fcontext -l | egrep -q -e '^/home'
   BrokenPipeError: [Errno 32] Broken pipe
   Exception ignored in: <_io.TextIOWrapper name='<stdout>' mode='w' encoding='UTF-8'>
   BrokenPipeError: [Errno 32] Broken pipe

Note that the error above only appears occasionally (usually only the
first line is printed).

Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
---
 python/semanage/semanage | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/python/semanage/semanage b/python/semanage/semanage
index b2fabea6..ce15983b 100644
--- a/python/semanage/semanage
+++ b/python/semanage/semanage
@@ -27,6 +27,7 @@ import traceback
 import argparse
 import seobject
 import sys
+import os
 PROGNAME = "policycoreutils"
 try:
     import gettext
@@ -945,6 +946,13 @@ def do_parser():
         args = commandParser.parse_args(make_args(sys.argv))
         args.func(args)
         sys.exit(0)
+    except BrokenPipeError as e:
+        sys.stderr.write("%s: %s\n" % (e.__class__.__name__, str(e)))
+        # Python flushes standard streams on exit; redirect remaining output
+        # to devnull to avoid another BrokenPipeError at shutdown
+        devnull = os.open(os.devnull, os.O_WRONLY)
+        os.dup2(devnull, sys.stdout.fileno())
+        sys.exit(1)
     except IOError as e:
         sys.stderr.write("%s: %s\n" % (e.__class__.__name__, str(e)))
         sys.exit(1)
-- 
2.29.2

Re: [PATCH] python/semanage: empty stdout before exiting on BrokenPipeError

[Nicolas Iooss]

On Tue, Jan 5, 2021 at 5:03 PM Vit Mojzis <vmojzis@redhat.com> wrote:
>
> Empty stdout buffer before exiting when BrokenPipeError is
> encountered. Otherwise python will flush the bufer during exit, which
> may trigger the exception again.
> https://docs.python.org/3/library/signal.html#note-on-sigpipe
>
> Fixes:
>    #semanage fcontext -l | egrep -q -e '^/home'
>    BrokenPipeError: [Errno 32] Broken pipe
>    Exception ignored in: <_io.TextIOWrapper name='<stdout>' mode='w' encoding='UTF-8'>
>    BrokenPipeError: [Errno 32] Broken pipe
>
> Note that the error above only appears occasionally (usually only the
> first line is printed).
>
> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> ---
>  python/semanage/semanage | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/python/semanage/semanage b/python/semanage/semanage
> index b2fabea6..ce15983b 100644
> --- a/python/semanage/semanage
> +++ b/python/semanage/semanage
> @@ -27,6 +27,7 @@ import traceback
>  import argparse
>  import seobject
>  import sys
> +import os

Hello,
It would be nicer if the imports were imported in alphabetical order
(also moving "import traceback" after "import sys"). But there also is
an "import re" in the middle of the file, and while at it the
Python2-compatibility layer in the gettext things could be dropped...
so this should be done in other patches anyway. No need to change this
patch.

>  PROGNAME = "policycoreutils"
>  try:
>      import gettext
> @@ -945,6 +946,13 @@ def do_parser():
>          args = commandParser.parse_args(make_args(sys.argv))
>          args.func(args)
>          sys.exit(0)
> +    except BrokenPipeError as e:
> +        sys.stderr.write("%s: %s\n" % (e.__class__.__name__, str(e)))
> +        # Python flushes standard streams on exit; redirect remaining output
> +        # to devnull to avoid another BrokenPipeError at shutdown
> +        devnull = os.open(os.devnull, os.O_WRONLY)
> +        os.dup2(devnull, sys.stdout.fileno())
> +        sys.exit(1)
>      except IOError as e:
>          sys.stderr.write("%s: %s\n" % (e.__class__.__name__, str(e)))
>          sys.exit(1)
> --
> 2.29.2

Doing open(os.devnull) + dup2 seems strange, but as this is precisely
what is documented in the official Python documentation that you
linked in the commit description, I am fine with this.

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Thanks!
Nicolas

[PATCH 2/2] python/semanage: Sort imports in alphabetical order

[Vit Mojzis]

Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
---
 python/semanage/semanage | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/python/semanage/semanage b/python/semanage/semanage
index ce15983b..125271df 100644
--- a/python/semanage/semanage
+++ b/python/semanage/semanage
@@ -23,11 +23,13 @@
 #
 #
 
-import traceback
 import argparse
+import os
+import re
 import seobject
 import sys
-import os
+import traceback
+
 PROGNAME = "policycoreutils"
 try:
     import gettext
@@ -798,8 +800,6 @@ def setupExportParser(subparsers):
     exportParser.add_argument('-f', '--output_file', dest='output_file', action=SetExportFile, help=_('Output file'))
     exportParser.set_defaults(func=handleExport)
 
-import re
-
 
 def mkargv(line):
     dquote = "\""
-- 
2.29.2

Re: [PATCH 2/2] python/semanage: Sort imports in alphabetical order

[Petr Lautrbach]

Vit Mojzis <vmojzis@redhat.com> writes:

> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>

Acked-by: Petr Lautrbach <plautrba@redhat.com>

Both merged, thanks!


> ---
>  python/semanage/semanage | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/python/semanage/semanage b/python/semanage/semanage
> index ce15983b..125271df 100644
> --- a/python/semanage/semanage
> +++ b/python/semanage/semanage
> @@ -23,11 +23,13 @@
>  #
>  #
>  
> -import traceback
>  import argparse
> +import os
> +import re
>  import seobject
>  import sys
> -import os
> +import traceback
> +
>  PROGNAME = "policycoreutils"
>  try:
>      import gettext
> @@ -798,8 +800,6 @@ def setupExportParser(subparsers):
>      exportParser.add_argument('-f', '--output_file', dest='output_file', action=SetExportFile, help=_('Output file'))
>      exportParser.set_defaults(func=handleExport)
>  
> -import re
> -
>  
>  def mkargv(line):
>      dquote = "\""
> -- 
> 2.29.2