Rev 4844: (spiv) Remove the SIGWINCH handler to avoid EINTR problems. Instead poll the in file:///home/pqm/archives/thelove/bzr/2.1/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu May 27 08:54:12 BST 2010
At file:///home/pqm/archives/thelove/bzr/2.1/
------------------------------------------------------------
revno: 4844 [merge]
revision-id: pqm at pqm.ubuntu.com-20100527075411-m4cm3or5cnu98mwu
parent: pqm at pqm.ubuntu.com-20100521024403-9usxn74f4nx1y3dm
parent: andrew.bennetts at canonical.com-20100527040001-gn0ryyuw1cf3z7ul
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.1
timestamp: Thu 2010-05-27 08:54:11 +0100
message:
(spiv) Remove the SIGWINCH handler to avoid EINTR problems. Instead poll the
terminal size as needed. (#583941) (Andrew Bennetts)
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/osutils.py osutils.py-20050309040759-eeaff12fbf77ac86
bzrlib/tests/test_osutils.py test_osutils.py-20051201224856-e48ee24c12182989
bzrlib/ui/text.py text.py-20051130153916-2e438cffc8afc478
=== modified file 'NEWS'
--- a/NEWS 2010-05-21 01:55:34 +0000
+++ b/NEWS 2010-05-26 02:19:33 +0000
@@ -20,6 +20,12 @@
* ``bzr switch`` does not die if a ConfigurableFileMerger is used.
(Aaron Bentley, #559436)
+* Do not register a SIGWINCH signal handler, instead just poll for the
+ terminal width as needed. This avoids the "Interrupted System Call"
+ problems that occur on POSIX with all currently released versions of
+ Python.
+ (Andrew Bennetts, #583941)
+
* Fixed ``AssertionError`` when accessing smart servers running Bazaar
versions before 1.6.
(Andrew Bennetts, #528041)
@@ -27,7 +33,7 @@
* Reset ``siginterrupt`` flag to False every time we handle a signal
installed with ``set_signal_handler(..., restart_syscall=True)`` (from
``bzrlib.osutils``. Reduces the likelihood of "Interrupted System Call"
- errors after two window resizes.
+ errors compared to registering ``signal.signal`` directly.
(Andrew Bennetts)
* Reduce peak memory by one copy of compressed text.
=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py 2010-04-30 06:27:15 +0000
+++ b/bzrlib/osutils.py 2010-05-27 04:00:01 +0000
@@ -1385,6 +1385,12 @@
terminal_width() returns None.
"""
+# Keep some state so that terminal_width can detect if _terminal_size has
+# returned a different size since the process started. See docstring and
+# comments of terminal_width for details.
+# _terminal_size_state has 3 possible values: no_data, unchanged, and changed.
+_terminal_size_state = 'no_data'
+_first_terminal_size = None
def terminal_width():
"""Return terminal width.
@@ -1394,20 +1400,34 @@
The rules are:
- if BZR_COLUMNS is set, returns its value
- if there is no controlling terminal, returns None
+ - query the OS, if the queried size has changed since the last query,
+ return its value,
- if COLUMNS is set, returns its value,
+ - if the OS has a value (even though it's never changed), return its value.
From there, we need to query the OS to get the size of the controlling
terminal.
- Unices:
+ On Unices we query the OS by:
- get termios.TIOCGWINSZ
- if an error occurs or a negative value is obtained, returns None
- Windows:
-
+ On Windows we query the OS by:
- win32utils.get_console_size() decides,
- returns None on error (provided default value)
"""
+ # Note to implementors: if changing the rules for determining the width,
+ # make sure you've considered the behaviour in these cases:
+ # - M-x shell in emacs, where $COLUMNS is set and TIOCGWINSZ returns 0,0.
+ # - bzr log | less, in bash, where $COLUMNS not set and TIOCGWINSZ returns
+ # 0,0.
+ # - (add more interesting cases here, if you find any)
+ # Some programs implement "Use $COLUMNS (if set) until SIGWINCH occurs",
+ # but we don't want to register a signal handler because it is impossible
+ # to do so without risking EINTR errors in Python <= 2.6.5 (see
+ # <http://bugs.python.org/issue8354>). Instead we check TIOCGWINSZ every
+ # time so we can notice if the reported size has changed, which should have
+ # a similar effect.
# If BZR_COLUMNS is set, take it, user is always right
try:
@@ -1416,24 +1436,39 @@
pass
isatty = getattr(sys.stdout, 'isatty', None)
- if isatty is None or not isatty():
+ if isatty is None or not isatty():
# Don't guess, setting BZR_COLUMNS is the recommended way to override.
return None
- # If COLUMNS is set, take it, the terminal knows better (even inside a
- # given terminal, the application can decide to set COLUMNS to a lower
- # value (splitted screen) or a bigger value (scroll bars))
+ # Query the OS
+ width, height = os_size = _terminal_size(None, None)
+ global _first_terminal_size, _terminal_size_state
+ if _terminal_size_state == 'no_data':
+ _first_terminal_size = os_size
+ _terminal_size_state = 'unchanged'
+ elif (_terminal_size_state == 'unchanged' and
+ _first_terminal_size != os_size):
+ _terminal_size_state = 'changed'
+
+ # If the OS claims to know how wide the terminal is, and this value has
+ # ever changed, use that.
+ if _terminal_size_state == 'changed':
+ if width is not None and width > 0:
+ return width
+
+ # If COLUMNS is set, use it.
try:
return int(os.environ['COLUMNS'])
except (KeyError, ValueError):
pass
- width, height = _terminal_size(None, None)
- if width <= 0:
- # Consider invalid values as meaning no width
- return None
+ # Finally, use an unchanged size from the OS, if we have one.
+ if _terminal_size_state == 'unchanged':
+ if width is not None and width > 0:
+ return width
- return width
+ # The width could not be determined.
+ return None
def _win32_terminal_size(width, height):
@@ -1466,29 +1501,6 @@
_terminal_size = _ioctl_terminal_size
-def _terminal_size_changed(signum, frame):
- """Set COLUMNS upon receiving a SIGnal for WINdow size CHange."""
- width, height = _terminal_size(None, None)
- if width is not None:
- os.environ['COLUMNS'] = str(width)
-
-
-_registered_sigwinch = False
-
-def watch_sigwinch():
- """Register for SIGWINCH, once and only once."""
- global _registered_sigwinch
- if not _registered_sigwinch:
- if sys.platform == 'win32':
- # Martin (gz) mentioned WINDOW_BUFFER_SIZE_RECORD from
- # ReadConsoleInput but I've no idea how to plug that in
- # the current design -- vila 20091216
- pass
- else:
- set_signal_handler(signal.SIGWINCH, _terminal_size_changed)
- _registered_sigwinch = True
-
-
def supports_executable():
return sys.platform != "win32"
=== modified file 'bzrlib/tests/test_osutils.py'
--- a/bzrlib/tests/test_osutils.py 2010-02-17 17:11:16 +0000
+++ b/bzrlib/tests/test_osutils.py 2010-05-27 04:00:01 +0000
@@ -1928,6 +1928,18 @@
class TestTerminalWidth(tests.TestCase):
+ def setUp(self):
+ tests.TestCase.setUp(self)
+ self._orig_terminal_size_state = osutils._terminal_size_state
+ self._orig_first_terminal_size = osutils._first_terminal_size
+ self.addCleanup(self.restore_osutils_globals)
+ osutils._terminal_size_state = 'no_data'
+ osutils._first_terminal_size = None
+
+ def restore_osutils_globals(self):
+ osutils._terminal_size_state = self._orig_terminal_size_state
+ osutils._first_terminal_size = self._orig_first_terminal_size
+
def replace_stdout(self, new):
orig_stdout = sys.stdout
def restore():
=== modified file 'bzrlib/ui/text.py'
--- a/bzrlib/ui/text.py 2010-03-23 06:45:56 +0000
+++ b/bzrlib/ui/text.py 2010-05-26 02:19:33 +0000
@@ -37,8 +37,6 @@
""")
-from bzrlib.osutils import watch_sigwinch
-
from bzrlib.ui import (
UIFactory,
NullProgressView,
@@ -62,8 +60,6 @@
self.stderr = stderr
# paints progress, network activity, etc
self._progress_view = self.make_progress_view()
- # hook up the signals to watch for terminal size changes
- watch_sigwinch()
def be_quiet(self, state):
if state and not self._quiet:
More information about the bazaar-commits
mailing list