Patchwork [3,of,3] sshserver: redirect stdin/stdout early and use duplicated streams

login
register
mail settings
Submitter Yuya Nishihara
Date May 7, 2018, 1:11 p.m.
Message ID <a7e53b70e5026bac9772.1525698684@mimosa>
Download mbox | patch
Permalink /patch/31301/
State Accepted
Headers show

Comments

Yuya Nishihara - May 7, 2018, 1:11 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1521964053 -32400
#      Sun Mar 25 16:47:33 2018 +0900
# Node ID a7e53b70e5026bac9772f9869e754a2a6f530587
# Parent  ea801aa5d559e37f68fb9bdda47dec6e58abcb88
sshserver: redirect stdin/stdout early and use duplicated streams

This is what we achieved with hook.redirect(True) plus ui.fout = ui.ferr.

The hook.redirect() function can't be completely removed yet since hgweb
still depends on it. I'm not sure if it is necessary for WSGI servers. CGI
needs it, but does WSGI communicate over stdin/stdout channels?
Matt Harbison - May 8, 2018, 4 a.m.
On Mon, 07 May 2018 09:11:24 -0400, Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1521964053 -32400
> #      Sun Mar 25 16:47:33 2018 +0900
> # Node ID a7e53b70e5026bac9772f9869e754a2a6f530587
> # Parent  ea801aa5d559e37f68fb9bdda47dec6e58abcb88
> sshserver: redirect stdin/stdout early and use duplicated streams
>
> This is what we achieved with hook.redirect(True) plus ui.fout = ui.ferr.
>
> The hook.redirect() function can't be completely removed yet since hgweb
> still depends on it. I'm not sure if it is necessary for WSGI servers.  
> CGI
> needs it, but does WSGI communicate over stdin/stdout channels?

Not sure if this is what you meant, but printing to stderr from  
hgweb/wireprotocol code landed in the error log for SCM Manager, which was  
nice.
Yuya Nishihara - May 8, 2018, 11:53 a.m.
On Tue, 08 May 2018 00:00:57 -0400, Matt Harbison wrote:
> On Mon, 07 May 2018 09:11:24 -0400, Yuya Nishihara <yuya@tcha.org> wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1521964053 -32400
> > #      Sun Mar 25 16:47:33 2018 +0900
> > # Node ID a7e53b70e5026bac9772f9869e754a2a6f530587
> > # Parent  ea801aa5d559e37f68fb9bdda47dec6e58abcb88
> > sshserver: redirect stdin/stdout early and use duplicated streams
> >
> > This is what we achieved with hook.redirect(True) plus ui.fout = ui.ferr.
> >
> > The hook.redirect() function can't be completely removed yet since hgweb
> > still depends on it. I'm not sure if it is necessary for WSGI servers.  
> > CGI
> > needs it, but does WSGI communicate over stdin/stdout channels?
> 
> Not sure if this is what you meant, but printing to stderr from  
> hgweb/wireprotocol code landed in the error log for SCM Manager, which was  
> nice.

stderr is unaffected by hook.redirect().

We know stdin/stdout is the IPC channel for SSH/CGI protocols so we need
either hook.redirect() or procutil.protectstdio(), but I have no idea about
the other types of WSGI frontends.
Augie Fackler - May 11, 2018, 4:41 a.m.
On Mon, May 07, 2018 at 10:11:24PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1521964053 -32400
> #      Sun Mar 25 16:47:33 2018 +0900
> # Node ID a7e53b70e5026bac9772f9869e754a2a6f530587
> # Parent  ea801aa5d559e37f68fb9bdda47dec6e58abcb88
> sshserver: redirect stdin/stdout early and use duplicated streams

queued these

>
> This is what we achieved with hook.redirect(True) plus ui.fout = ui.ferr.
>
> The hook.redirect() function can't be completely removed yet since hgweb
> still depends on it. I'm not sure if it is necessary for WSGI servers. CGI
> needs it, but does WSGI communicate over stdin/stdout channels?

I don't think it's required, but it's at least polite to not spew
things on stderr etc in WSGI.

Patch

diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -18,7 +18,6 @@  from .thirdparty import (
 from . import (
     encoding,
     error,
-    hook,
     pycompat,
     util,
     wireprototypes,
@@ -785,8 +784,7 @@  class sshserver(object):
     def __init__(self, ui, repo, logfh=None):
         self._ui = ui
         self._repo = repo
-        self._fin = ui.fin
-        self._fout = ui.fout
+        self._fin, self._fout = procutil.protectstdio(ui.fin, ui.fout)
 
         # Log write I/O to stdout and stderr if configured.
         if logfh:
@@ -795,11 +793,10 @@  class sshserver(object):
             ui.ferr = util.makeloggingfileobject(
                 logfh, ui.ferr, 'e', logdata=True)
 
-        hook.redirect(True)
-        ui.fout = repo.ui.fout = ui.ferr
-
     def serve_forever(self):
         self.serveuntil(threading.Event())
+        procutil.restorestdio(self._ui.fin, self._ui.fout,
+                              self._fin, self._fout)
         sys.exit(0)
 
     def serveuntil(self, ev):