Permit kernel std* to be redirected #420

Merged
merged 4 commits into from May 11, 2011
@@ -23,6 +23,7 @@
from parentpoller import ParentPollerUnix, ParentPollerWindows
from session import Session
+
def bind_port(socket, ip, port):
""" Binds the specified ZMQ socket. If the port is zero, a random port is
chosen. Returns the port that was bound.
@@ -51,6 +52,10 @@ def make_argument_parser():
help='set the REQ channel port [default: random]')
parser.add_argument('--hb', type=int, metavar='PORT', default=0,
help='set the heartbeat port [default: random]')
+ parser.add_argument('--no-stdout', action='store_true',
+ help='redirect stdout to the null device')
+ parser.add_argument('--no-stderr', action='store_true',
+ help='redirect stderr to the null device')
if sys.platform == 'win32':
parser.add_argument('--interrupt', type=int, metavar='HANDLE',
@@ -71,13 +76,13 @@ def make_kernel(namespace, kernel_factory,
""" Creates a kernel, redirects stdout/stderr, and installs a display hook
and exception handler.
"""
- # If running under pythonw.exe, the interpreter will crash if more than 4KB
- # of data is written to stdout or stderr. This is a bug that has been with
- # Python for a very long time; see http://bugs.python.org/issue706263.
- if sys.executable.endswith('pythonw.exe'):
+ # Re-direct stdout/stderr, if necessary.
+ if namespace.no_stdout or namespace.no_stderr:
blackhole = file(os.devnull, 'w')
- sys.stdout = sys.stderr = blackhole
- sys.__stdout__ = sys.__stderr__ = blackhole
+ if namespace.no_stdout:
+ sys.stdout = sys.__stdout__ = blackhole
@takluyver
takluyver May 9, 2011 Member

According to the docs (http://docs.python.org/library/sys.html#sys.__stdin__), the double-underscored versions of the names (sys.__stdout__ etc.) are supposed to preserve the original stdin/stdout/stderr for anything that explicitly wants to access them later. What's our rationale for overriding those here, in addition to the 'working locations' (sys.stdout etc.)?

@takluyver
takluyver May 9, 2011 Member

P.S. I recognise that we were already doing that before this pull request. I'm just interested to understand it.

@epatters
epatters May 9, 2011 Contributor

The rational is that the sys.__stdout__ and sys.__stderr__ created for pythonw.exe are broken. If you write more than 4096 bytes to them, the interpreter will crash. Nice, huh? There is no reason to preserve them--you don't want them.

@takluyver
takluyver May 9, 2011 Member

...In one go? Or as soon as you get to 4096 bytes, it crashes?

So does this bit of the code only get executed under pythonw.exe? I see the check for the executable file has gone, is one of the conditionals equivalent?

@epatters
epatters May 10, 2011 Contributor

As soon as you hit 4096 bytes. See http://bugs.python.org/issue706263 for more information about the bug.

This bit of code is only executed under pythonw.exe and only when the streams are not explictly redirected by the kernel launch process. The latter condition is the reason that I can't leave the check for pythonw.exe in the kernel process itself: there is no way, as far as I can tell, for the kernel process to know if it has a '"bad" stdout or stderr. I am using the --no-stdout and --no-stderr options to communicate that information.

@epatters
epatters May 10, 2011 Contributor

To be clear: the reason that I was able to do the check for pythonw.exe in the kernel process before was that there was no option to redirect the kernel's stdout or stderr. I could do whatever I wanted with them.

+ if namespace.no_stderr:
+ sys.stderr = sys.__stderr__ = blackhole
# Install minimal exception handling
sys.excepthook = FormattedTB(mode='Verbose', color_scheme='NoColor',
@@ -155,6 +160,7 @@ def main():
def base_launch_kernel(code, xrep_port=0, pub_port=0, req_port=0, hb_port=0,
+ stdin=None, stdout=None, stderr=None,
executable=None, independent=False, extra_arguments=[]):
""" Launches a localhost kernel, binding to the specified ports.
@@ -175,6 +181,9 @@ def base_launch_kernel(code, xrep_port=0, pub_port=0, req_port=0, hb_port=0,
hb_port : int, optional
The port to use for the hearbeat REP channel.
+ stdin, stdout, stderr : optional (default None)
+ Standards streams, as defined in subprocess.Popen.
+
executable : str, optional (default sys.executable)
The Python executable to use for the kernel process.
@@ -228,15 +237,35 @@ def base_launch_kernel(code, xrep_port=0, pub_port=0, req_port=0, hb_port=0,
interrupt_event = ParentPollerWindows.create_interrupt_event()
arguments += [ '--interrupt', str(int(interrupt_event)) ]
- # If using pythonw, stdin, stdout, and stderr are invalid. Popen will
- # fail unless they are suitably redirected. We don't read from the
- # pipes, but they must exist.
- redirect = PIPE if executable.endswith('pythonw.exe') else None
-
+ # If this process in running on pythonw, stdin, stdout, and stderr are
+ # invalid. Popen will fail unless they are suitably redirected. We don't
+ # read from the pipes, but they must exist.
+ if sys.executable.endswith('pythonw.exe'):
+ redirect = True
+ _stdin = PIPE if stdin is None else stdin
+ _stdout = PIPE if stdout is None else stdout
+ _stderr = PIPE if stderr is None else stderr
+ else:
+ redirect = False
+ _stdin, _stdout, _stderr = stdin, stdout, stderr
+
+ # If the kernel is running on pythonw and stdout/stderr are not been
+ # re-directed, it will crash when more than 4KB of data is written to
+ # stdout or stderr. This is a bug that has been with Python for a very
+ # long time; see http://bugs.python.org/issue706263.
+ # A cleaner solution to this problem would be to pass os.devnull to
+ # Popen directly. Unfortunately, that does not work.
+ if executable.endswith('pythonw.exe'):
+ if stdout is None:
+ arguments.append('--no-stdout')
+ if stderr is None:
+ arguments.append('--no-stderr')
+
+ # Launch the kernel process.
if independent:
proc = Popen(arguments,
creationflags=512, # CREATE_NEW_PROCESS_GROUP
- stdout=redirect, stderr=redirect, stdin=redirect)
+ stdin=_stdin, stdout=_stdout, stderr=_stderr)
else:
from _subprocess import DuplicateHandle, GetCurrentProcess, \
DUPLICATE_SAME_ACCESS
@@ -245,21 +274,26 @@ def base_launch_kernel(code, xrep_port=0, pub_port=0, req_port=0, hb_port=0,
True, # Inheritable by new processes.
DUPLICATE_SAME_ACCESS)
proc = Popen(arguments + ['--parent', str(int(handle))],
- stdout=redirect, stderr=redirect, stdin=redirect)
+ stdin=_stdin, stdout=_stdout, stderr=_stderr)
# Attach the interrupt event to the Popen objet so it can be used later.
proc.win32_interrupt_event = interrupt_event
# Clean up pipes created to work around Popen bug.
- if redirect is not None:
- proc.stdout.close()
- proc.stderr.close()
- proc.stdin.close()
+ if redirect:
+ if stdin is None:
+ proc.stdin.close()
+ if stdout is None:
+ proc.stdout.close()
+ if stderr is None:
+ proc.stderr.close()
else:
if independent:
- proc = Popen(arguments, preexec_fn=lambda: os.setsid())
+ proc = Popen(arguments, preexec_fn=lambda: os.setsid(),
+ stdin=stdin, stdout=stdout, stderr=stderr)
else:
- proc = Popen(arguments + ['--parent'])
+ proc = Popen(arguments + ['--parent'],
+ stdin=stdin, stdout=stdout, stderr=stderr)
return proc, xrep_port, pub_port, req_port, hb_port
@@ -551,6 +551,7 @@ def start(self):
#-----------------------------------------------------------------------------
def launch_kernel(ip=None, xrep_port=0, pub_port=0, req_port=0, hb_port=0,
+ stdin=None, stdout=None, stderr=None,
executable=None, independent=False, pylab=False, colors=None):
"""Launches a localhost kernel, binding to the specified ports.
@@ -571,6 +572,9 @@ def launch_kernel(ip=None, xrep_port=0, pub_port=0, req_port=0, hb_port=0,
hb_port : int, optional
The port to use for the hearbeat REP channel.
+ stdin, stdout, stderr : optional (default None)
+ Standards streams, as defined in subprocess.Popen.
+
executable : str, optional (default sys.executable)
The Python executable to use for the kernel process.
@@ -608,6 +612,7 @@ def launch_kernel(ip=None, xrep_port=0, pub_port=0, req_port=0, hb_port=0,
extra_arguments.append(colors)
return base_launch_kernel('from IPython.zmq.ipkernel import main; main()',
xrep_port, pub_port, req_port, hb_port,
+ stdin, stdout, stderr,
executable, independent, extra_arguments)
@@ -573,7 +573,8 @@ def run(self):
# list, poll is working correctly even if it
# returns quickly. Note: poll timeout is in
# milliseconds.
- self.poller.poll(1000*until_dead)
+ if until_dead > 0.0:
+ self.poller.poll(1000 * until_dead)
since_last_heartbeat = time.time()-request_time
if since_last_heartbeat > self.time_to_dead:
@@ -835,8 +836,15 @@ def kill_kernel(self):
except OSError, e:
# In Windows, we will get an Access Denied error if the process
# has already terminated. Ignore it.
- if not (sys.platform == 'win32' and e.winerror == 5):
- raise
+ if sys.platform == 'win32':
+ if e.winerror != 5:
+ raise
+ # On Unix, we may get an ESRCH error if the process has already
+ # terminated. Ignore it.
+ else:
+ from errno import ESRCH
+ if e.errno != ESRCH:
+ raise
self.kernel = None
else:
raise RuntimeError("Cannot kill kernel. No kernel is running!")
@@ -248,6 +248,7 @@ def _symbol_from_context(self, context):
#-----------------------------------------------------------------------------
def launch_kernel(ip=None, xrep_port=0, pub_port=0, req_port=0, hb_port=0,
+ stdin=None, stdout=None, stderr=None,
executable=None, independent=False):
""" Launches a localhost kernel, binding to the specified ports.
@@ -268,6 +269,9 @@ def launch_kernel(ip=None, xrep_port=0, pub_port=0, req_port=0, hb_port=0,
hb_port : int, optional
The port to use for the hearbeat REP channel.
+ stdin, stdout, stderr : optional (default None)
+ Standards streams, as defined in subprocess.Popen.
+
executable : str, optional (default sys.executable)
The Python executable to use for the kernel process.
@@ -291,8 +295,8 @@ def launch_kernel(ip=None, xrep_port=0, pub_port=0, req_port=0, hb_port=0,
return base_launch_kernel('from IPython.zmq.pykernel import main; main()',
xrep_port, pub_port, req_port, hb_port,
- executable, independent,
- extra_arguments=extra_arguments)
+ stdin, stdout, stderr,
+ executable, independent, extra_arguments)
main = make_default_main(Kernel)