[issue12196] add pipe2() to the os module

Charles-François Natali report at bugs.python.org
Sat May 28 19:45:42 CEST 2011


Charles-François Natali <neologix at free.fr> added the comment:

> I just nuked the pure Python POSIX subprocess implementation

Nice.

I've updated both patches to address Victor's comments on test_io and
test_subprocess usage of PIPE_MAX_SIZE, and Ross' comment on pipe2's
configure tests.
I left subprocess use _posixsubprocess.cloexec_pipe, since it leads to
simpler code, especially now that the Python version has been removed.

----------
Added file: http://bugs.python.org/file22166/posix_pipe2.diff
Added file: http://bugs.python.org/file22167/support_pipe_max.diff

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue12196>
_______________________________________
-------------- next part --------------
diff -r c5bd972391cd Doc/library/os.rst
--- a/Doc/library/os.rst	Sat May 28 10:00:14 2011 -0700
+++ b/Doc/library/os.rst	Sat May 28 19:07:31 2011 +0200
@@ -1019,6 +1019,17 @@
    Availability: Unix, Windows.
 
 
+.. function:: pipe2(flags=0)
+
+   Create a pipe with *flags* set atomically.
+   *flags* is optional and can be constructed by ORing together zero or more of
+   these values: :data:`O_NONBLOCK`, :data:`O_CLOEXEC`.
+   Return a pair of file descriptors ``(r, w)`` usable for reading and writing,
+   respectively.
+
+   Availability: some flavors of Unix.
+
+
 .. function:: posix_fallocate(fd, offset, len)
 
    Ensures that enough disk space is allocated for the file specified by *fd*
diff -r c5bd972391cd Lib/test/test_posix.py
--- a/Lib/test/test_posix.py	Sat May 28 10:00:14 2011 -0700
+++ b/Lib/test/test_posix.py	Sat May 28 19:07:31 2011 +0200
@@ -477,6 +477,31 @@
             reader, writer = posix.pipe()
             os.close(reader)
             os.close(writer)
+            
+    @unittest.skipUnless(hasattr(os, 'pipe2'), "test needs os.pipe2()")
+    def test_pipe2(self):
+        self.assertRaises(TypeError, os.pipe2, 'DEADBEEF')
+        self.assertRaises(TypeError, os.pipe2, 0, 0)
+
+        # try calling without flag, like os.pipe()
+        r, w = os.pipe2()
+        os.close(r)
+        os.close(w)
+
+        # test flags
+        r, w = os.pipe2(os.O_CLOEXEC|os.O_NONBLOCK)
+        self.addCleanup(os.close, r)
+        self.addCleanup(os.close, w)
+        self.assertTrue(fcntl.fcntl(r, fcntl.F_GETFD) & fcntl.FD_CLOEXEC)
+        self.assertTrue(fcntl.fcntl(w, fcntl.F_GETFD) & fcntl.FD_CLOEXEC)
+        # try reading from an empty pipe: this should fail, not block
+        self.assertRaises(OSError, os.read, r, 1)
+        # try a write big enough to fill-up the pipe (64K on most kernels): this
+        # should either fail or perform a partial write, not block
+        try:
+            os.write(w, b'x' * support.PIPE_MAX_SIZE)
+        except OSError:
+            pass
 
     def test_utime(self):
         if hasattr(posix, 'utime'):
diff -r c5bd972391cd Modules/posixmodule.c
--- a/Modules/posixmodule.c	Sat May 28 10:00:14 2011 -0700
+++ b/Modules/posixmodule.c	Sat May 28 19:07:31 2011 +0200
@@ -6547,6 +6547,31 @@
 }
 #endif  /* HAVE_PIPE */
 
+#ifdef HAVE_PIPE2
+PyDoc_STRVAR(posix_pipe2__doc__,
+"pipe2(flags=0) -> (read_end, write_end)\n\n\
+Create a pipe with flags set atomically.\
+flags is optional and can be constructed by ORing together zero or more\n\
+of these values: O_NONBLOCK, O_CLOEXEC.\n\
+");
+
+static PyObject *
+posix_pipe2(PyObject *self, PyObject *args)
+{
+    int flags = 0;
+    int fds[2];
+    int res;
+
+    if (!PyArg_ParseTuple(args, "|i:pipe2", &flags))
+        return NULL;
+
+    res = pipe2(fds, flags);
+    if (res != 0)
+        return posix_error();
+    return Py_BuildValue("(ii)", fds[0], fds[1]);
+}
+#endif /* HAVE_PIPE2 */
+
 #ifdef HAVE_WRITEV
 PyDoc_STRVAR(posix_writev__doc__,
 "writev(fd, buffers) -> byteswritten\n\n\
@@ -9441,6 +9466,9 @@
 #ifdef HAVE_PIPE
     {"pipe",            posix_pipe, METH_NOARGS, posix_pipe__doc__},
 #endif
+#ifdef HAVE_PIPE2
+    {"pipe2",           posix_pipe2, METH_VARARGS, posix_pipe2__doc__},
+#endif
 #ifdef HAVE_MKFIFO
     {"mkfifo",          posix_mkfifo, METH_VARARGS, posix_mkfifo__doc__},
 #endif
diff -r c5bd972391cd configure
--- a/configure	Sat May 28 10:00:14 2011 -0700
+++ b/configure	Sat May 28 19:07:31 2011 +0200
@@ -9307,7 +9307,7 @@
  getpriority getresuid getresgid getpwent getspnam getspent getsid getwd \
  if_nameindex \
  initgroups kill killpg lchmod lchown lockf linkat lstat lutimes mbrtowc mkdirat mkfifo \
- mkfifoat mknod mknodat mktime mremap nice openat pathconf pause plock poll \
+ mkfifoat mknod mknodat mktime mremap nice openat pathconf pause pipe2 plock poll \
  posix_fallocate posix_fadvise pread \
  pthread_init pthread_kill putenv pwrite readlink readlinkat readv realpath renameat \
  select sem_open sem_timedwait sem_getvalue sem_unlink sendfile setegid seteuid \
@@ -13824,14 +13824,6 @@
 esac
 
 
-ac_fn_c_check_func "$LINENO" "pipe2" "ac_cv_func_pipe2"
-if test "x$ac_cv_func_pipe2" = x""yes; then :
-
-$as_echo "#define HAVE_PIPE2 1" >>confdefs.h
-
-fi
-
-
 
 
 for h in `(cd $srcdir;echo Python/thread_*.h)`
diff -r c5bd972391cd configure.in
--- a/configure.in	Sat May 28 10:00:14 2011 -0700
+++ b/configure.in	Sat May 28 19:07:31 2011 +0200
@@ -2531,7 +2531,7 @@
  getpriority getresuid getresgid getpwent getspnam getspent getsid getwd \
  if_nameindex \
  initgroups kill killpg lchmod lchown lockf linkat lstat lutimes mbrtowc mkdirat mkfifo \
- mkfifoat mknod mknodat mktime mremap nice openat pathconf pause plock poll \
+ mkfifoat mknod mknodat mktime mremap nice openat pathconf pause pipe2 plock poll \
  posix_fallocate posix_fadvise pread \
  pthread_init pthread_kill putenv pwrite readlink readlinkat readv realpath renameat \
  select sem_open sem_timedwait sem_getvalue sem_unlink sendfile setegid seteuid \
@@ -4244,8 +4244,6 @@
 esac
 
 
-AC_CHECK_FUNC(pipe2, AC_DEFINE(HAVE_PIPE2, 1, [Define if the OS supports pipe2()]), )
-
 AC_SUBST(THREADHEADERS)
 
 for h in `(cd $srcdir;echo Python/thread_*.h)`
diff -r c5bd972391cd pyconfig.h.in
--- a/pyconfig.h.in	Sat May 28 10:00:14 2011 -0700
+++ b/pyconfig.h.in	Sat May 28 19:07:31 2011 +0200
@@ -560,7 +560,7 @@
 /* Define to 1 if you have the `pause' function. */
 #undef HAVE_PAUSE
 
-/* Define if the OS supports pipe2() */
+/* Define to 1 if you have the `pipe2' function. */
 #undef HAVE_PIPE2
 
 /* Define to 1 if you have the `plock' function. */
-------------- next part --------------
diff -r d0bc18a50bd1 Lib/test/support.py
--- a/Lib/test/support.py	Sat May 28 03:09:33 2011 -0700
+++ b/Lib/test/support.py	Sat May 28 18:59:15 2011 +0200
@@ -48,7 +48,7 @@
     "threading_cleanup", "reap_children", "cpython_only", "check_impl_detail",
     "get_attribute", "swap_item", "swap_attr", "requires_IEEE_754",
     "TestHandler", "Matcher", "can_symlink", "skip_unless_symlink",
-    "import_fresh_module", "requires_zlib"
+    "import_fresh_module", "requires_zlib", "PIPE_MAX_SIZE"
     ]
 
 class Error(Exception):
@@ -409,6 +409,13 @@
 
 IPV6_ENABLED = _is_ipv6_enabled()
 
+
+# A constant likely larger than the underlying OS pipe buffer size.
+# Windows limit seems to be around 512B, and most Unix kernels have a 64K pipe
+# buffer size: take 1M to be sure.
+PIPE_MAX_SIZE = 1024 * 1024
+
+
 # decorator for skipping tests on non-IEEE 754 platforms
 requires_IEEE_754 = unittest.skipUnless(
     float.__getformat__("double").startswith("IEEE"),
diff -r d0bc18a50bd1 Lib/test/test_io.py
--- a/Lib/test/test_io.py	Sat May 28 03:09:33 2011 -0700
+++ b/Lib/test/test_io.py	Sat May 28 18:59:15 2011 +0200
@@ -2683,7 +2683,7 @@
             # The buffered IO layer must check for pending signal
             # handlers, which in this case will invoke alarm_interrupt().
             self.assertRaises(ZeroDivisionError,
-                              wio.write, item * (1024 * 1024))
+                        wio.write, item * (support.PIPE_MAX_SIZE // len(item)))
             t.join()
             # We got one byte, get another one and check that it isn't a
             # repeat of the first one.
diff -r d0bc18a50bd1 Lib/test/test_subprocess.py
--- a/Lib/test/test_subprocess.py	Sat May 28 03:09:33 2011 -0700
+++ b/Lib/test/test_subprocess.py	Sat May 28 18:59:15 2011 +0200
@@ -489,24 +489,21 @@
         # This test will probably deadlock rather than fail, if
         # communicate() does not work properly.
         x, y = os.pipe()
-        if mswindows:
-            pipe_buf = 512
-        else:
-            pipe_buf = os.fpathconf(x, "PC_PIPE_BUF")
         os.close(x)
         os.close(y)
         p = subprocess.Popen([sys.executable, "-c",
                               'import sys,os;'
                               'sys.stdout.write(sys.stdin.read(47));'
-                              'sys.stderr.write("xyz"*%d);'
-                              'sys.stdout.write(sys.stdin.read())' % pipe_buf],
+                              'sys.stderr.write("x" * %d);'
+                              'sys.stdout.write(sys.stdin.read())' %
+                              support.PIPE_MAX_SIZE],
                              stdin=subprocess.PIPE,
                              stdout=subprocess.PIPE,
                              stderr=subprocess.PIPE)
         self.addCleanup(p.stdout.close)
         self.addCleanup(p.stderr.close)
         self.addCleanup(p.stdin.close)
-        string_to_write = b"abc"*pipe_buf
+        string_to_write = b"a" * support.PIPE_MAX_SIZE
         (stdout, stderr) = p.communicate(string_to_write)
         self.assertEqual(stdout, string_to_write)
 


More information about the Python-bugs-list mailing list