Patchwork [V2] posix: insert seek between reads and writes on Solaris (issue4943)

login
register
mail settings
Submitter Gregory Szorc
Date Nov. 27, 2015, 2:10 a.m.
Message ID <a7d5be598b347ae2c265.1448590231@ubuntu-main>
Download mbox | patch
Permalink /patch/11665/
State Rejected
Delegated to: Matt Mackall
Headers show

Comments

Gregory Szorc - Nov. 27, 2015, 2:10 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1447625312 28800
#      Sun Nov 15 14:08:32 2015 -0800
# Branch stable
# Node ID a7d5be598b347ae2c26566aa2bcdcf5cb1acb30b
# Parent  6979fe2a6d75105affcacd9e298262a92641cb98
posix: insert seek between reads and writes on Solaris (issue4943)

At least some versions of Solaris fail to properly write to file
descriptors opened for both reading and writing. When the descriptor
is seeked and read, subsequent writes effectively disappear into a
black hole.

The underlying cause of this is unknown. Some versions of Solaris
appear to work. Others don't. Read the issue comments for more
details.

The issue can be worked around by inserting an extra seek between
read and write operations. This exact workaround is already in place
for Windows, where the MSDN docs call out this requirement. See
3686fa2b8eee.

This patch moves the existing workaround code into util.py and
moves the wrapping of posixfile instances on Windows and Solaris
into util.py as well.

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -8,16 +8,17 @@ 
 # GNU General Public License version 2 or any later version.
 
 """Mercurial utility functions and platform specific implementations.
 
 This contains helper routines that are independent of the SCM core and
 hide platform-specific details from the core.
 """
 
+import platform as pyplatform
 import i18n
 _ = i18n._
 import error, osutil, encoding, parsers
 import errno, shutil, sys, tempfile, traceback
 import re as remod
 import os, time, datetime, calendar, textwrap, signal, collections
 import stat
 import imp, socket, urllib
@@ -54,17 +55,16 @@  normcase = platform.normcase
 normcasespec = platform.normcasespec
 normcasefallback = platform.normcasefallback
 openhardlinks = platform.openhardlinks
 oslink = platform.oslink
 parsepatchoutput = platform.parsepatchoutput
 pconvert = platform.pconvert
 poll = platform.poll
 popen = platform.popen
-posixfile = platform.posixfile
 quotecommand = platform.quotecommand
 readpipe = platform.readpipe
 rename = platform.rename
 removedirs = platform.removedirs
 samedevice = platform.samedevice
 samefile = platform.samefile
 samestat = platform.samestat
 setbinary = platform.setbinary
@@ -801,16 +801,98 @@  def checksignature(func):
             return func(*args, **kwargs)
         except TypeError:
             if len(traceback.extract_tb(sys.exc_info()[2])) == 1:
                 raise error.SignatureError
             raise
 
     return check
 
+class mixedfilemodewrapper(object):
+    """Wraps a file handle when it is opened in read/write mode.
+
+    fopen() and fdopen() on Windows have a specific-to-Windows requirement
+    that files opened with mode r+, w+, or a+ make a call to a file positioning
+    function when switching between reads and writes. Without this extra call,
+    Python will raise a not very intuitive "IOError: [Errno 0] Error."
+
+    Solaris appears to suffer from a similar issue, although it will lose
+    its file seek position instead of raising an error. See issue4943.
+    This issue appears to be fixed in Python 3 on Solaris.
+
+    This class wraps posixfile instances when the file is opened in read/write
+    mode and automatically adds checks or inserts appropriate file positioning
+    calls when necessary.
+    """
+    OPNONE = 0
+    OPREAD = 1
+    OPWRITE = 2
+
+    def __init__(self, fp):
+        object.__setattr__(self, '_fp', fp)
+        object.__setattr__(self, '_lastop', 0)
+
+    def __getattr__(self, name):
+        return getattr(self._fp, name)
+
+    def __setattr__(self, name, value):
+        return self._fp.__setattr__(name, value)
+
+    def _noopseek(self):
+        self._fp.seek(0, os.SEEK_CUR)
+
+    def seek(self, *args, **kwargs):
+        object.__setattr__(self, '_lastop', self.OPNONE)
+        return self._fp.seek(*args, **kwargs)
+
+    def write(self, d):
+        if self._lastop == self.OPREAD:
+            self._noopseek()
+
+        object.__setattr__(self, '_lastop', self.OPWRITE)
+        return self._fp.write(d)
+
+    def writelines(self, *args, **kwargs):
+        if self._lastop == self.OPREAD:
+            self._noopeseek()
+
+        object.__setattr__(self, '_lastop', self.OPWRITE)
+        return self._fp.writelines(*args, **kwargs)
+
+    def read(self, *args, **kwargs):
+        if self._lastop == self.OPWRITE:
+            self._noopseek()
+
+        object.__setattr__(self, '_lastop', self.OPREAD)
+        return self._fp.read(*args, **kwargs)
+
+    def readline(self, *args, **kwargs):
+        if self._lastop == self.OPWRITE:
+            self._noopseek()
+
+        object.__setattr__(self, '_lastop', self.OPREAD)
+        return self._fp.readline(*args, **kwargs)
+
+    def readlines(self, *args, **kwargs):
+        if self._lastop == self.OPWRITE:
+            self._noopseek()
+
+        object.__setattr__(self, '_lastop', self.OPREAD)
+        return self._fp.readlines(*args, **kwargs)
+
+if os.name == 'nt' or pyplatform.system() == 'SunOS':
+    def posixfile(name, mode='r', buffering=-1):
+        fp = platform.posixfile(name, mode, buffering)
+
+        if '+' in mode:
+            return mixedfilemodewrapper(fp)
+        return fp
+else:
+    posixfile = platform.posixfile
+
 def copyfile(src, dest, hardlink=False):
     "copy a file, preserving mode and atime/mtime"
     if os.path.lexists(dest):
         unlink(dest)
     # hardlinks are problematic on CIFS, quietly ignore this flag
     # until we find a way to work around it cleanly (issue4546)
     if False and hardlink:
         try:
diff --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -22,97 +22,26 @@  setsignalhandler = win32.setsignalhandle
 spawndetached = win32.spawndetached
 split = os.path.split
 termwidth = win32.termwidth
 testpid = win32.testpid
 unlink = win32.unlink
 
 umask = 0o022
 
-class mixedfilemodewrapper(object):
-    """Wraps a file handle when it is opened in read/write mode.
-
-    fopen() and fdopen() on Windows have a specific-to-Windows requirement
-    that files opened with mode r+, w+, or a+ make a call to a file positioning
-    function when switching between reads and writes. Without this extra call,
-    Python will raise a not very intuitive "IOError: [Errno 0] Error."
-
-    This class wraps posixfile instances when the file is opened in read/write
-    mode and automatically adds checks or inserts appropriate file positioning
-    calls when necessary.
-    """
-    OPNONE = 0
-    OPREAD = 1
-    OPWRITE = 2
-
-    def __init__(self, fp):
-        object.__setattr__(self, '_fp', fp)
-        object.__setattr__(self, '_lastop', 0)
-
-    def __getattr__(self, name):
-        return getattr(self._fp, name)
-
-    def __setattr__(self, name, value):
-        return self._fp.__setattr__(name, value)
-
-    def _noopseek(self):
-        self._fp.seek(0, os.SEEK_CUR)
-
-    def seek(self, *args, **kwargs):
-        object.__setattr__(self, '_lastop', self.OPNONE)
-        return self._fp.seek(*args, **kwargs)
-
-    def write(self, d):
-        if self._lastop == self.OPREAD:
-            self._noopseek()
-
-        object.__setattr__(self, '_lastop', self.OPWRITE)
-        return self._fp.write(d)
-
-    def writelines(self, *args, **kwargs):
-        if self._lastop == self.OPREAD:
-            self._noopeseek()
-
-        object.__setattr__(self, '_lastop', self.OPWRITE)
-        return self._fp.writelines(*args, **kwargs)
-
-    def read(self, *args, **kwargs):
-        if self._lastop == self.OPWRITE:
-            self._noopseek()
-
-        object.__setattr__(self, '_lastop', self.OPREAD)
-        return self._fp.read(*args, **kwargs)
-
-    def readline(self, *args, **kwargs):
-        if self._lastop == self.OPWRITE:
-            self._noopseek()
-
-        object.__setattr__(self, '_lastop', self.OPREAD)
-        return self._fp.readline(*args, **kwargs)
-
-    def readlines(self, *args, **kwargs):
-        if self._lastop == self.OPWRITE:
-            self._noopseek()
-
-        object.__setattr__(self, '_lastop', self.OPREAD)
-        return self._fp.readlines(*args, **kwargs)
-
 def posixfile(name, mode='r', buffering=-1):
     '''Open a file with even more POSIX-like semantics'''
     try:
         fp = osutil.posixfile(name, mode, buffering) # may raise WindowsError
 
         # The position when opening in append mode is implementation defined, so
         # make it consistent with other platforms, which position at EOF.
         if 'a' in mode:
             fp.seek(0, os.SEEK_END)
 
-        if '+' in mode:
-            return mixedfilemodewrapper(fp)
-
         return fp
     except WindowsError as err:
         # convert to a friendlier exception
         raise IOError(err.errno, '%s: %s' % (name, err.strerror))
 
 class winstdout(object):
     '''stdout on windows misbehaves if sent through a pipe'''