[Jython-checkins] jython (merge default -> default): Merge _io open and close improvements
jeff.allen
jython-checkins at python.org
Mon Jan 28 00:25:16 CET 2013
http://hg.python.org/jython/rev/98b9c3a9076e
changeset: 6961:98b9c3a9076e
parent: 6958:0f6f1d4077c4
parent: 6960:68c3d85a3a6b
user: Jeff Allen <ja...py at farowl.co.uk>
date: Sun Jan 27 22:42:20 2013 +0000
summary:
Merge _io open and close improvements
files:
src/org/python/core/PySystemState.java | 57 ++-
src/org/python/modules/_io/Closer.java | 14 +-
src/org/python/modules/_io/PyFileIO.java | 16 +-
src/org/python/modules/_io/_io.java | 121 +++--
tests/java/org/python/modules/_io/_ioTest.java | 178 +++++++++-
5 files changed, 302 insertions(+), 84 deletions(-)
diff --git a/src/org/python/core/PySystemState.java b/src/org/python/core/PySystemState.java
--- a/src/org/python/core/PySystemState.java
+++ b/src/org/python/core/PySystemState.java
@@ -17,7 +17,9 @@
import java.nio.charset.Charset;
import java.nio.charset.UnsupportedCharsetException;
import java.security.AccessControlException;
+import java.util.Iterator;
import java.util.LinkedHashSet;
+import java.util.LinkedList;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
@@ -1382,14 +1384,13 @@
}
}
- for (Callable<Void> callable : resourceClosers) {
- try {
- callable.call();
- } catch (Exception e) {
- // just continue, nothing we can do
- }
- }
+ // Close the listed resources (and clear the list)
+ runClosers(resourceClosers);
resourceClosers.clear();
+
+ // XXX Not sure this is ok, but it makes repeat testing possible.
+ // Re-enable the management of resource closers
+ isCleanup = false;
}
// Python scripts expect that files are closed upon an orderly cleanup of the VM.
@@ -1412,22 +1413,42 @@
@Override
public synchronized void run() {
- if (resourceClosers == null) {
- // resourceClosers can be null in some strange cases
- return;
- }
- for (Callable<Void> callable : resourceClosers) {
- try {
- callable.call(); // side effect of being removed from this set
- } catch (Exception e) {
- // continue - nothing we can do now!
- }
- }
+ runClosers(resourceClosers);
resourceClosers.clear();
}
}
}
+
+ /**
+ * Helper abstracting common code from {@link ShutdownCloser#run()} and
+ * {@link PySystemStateCloser#cleanup()} to close resources (such as still-open files). The
+ * closing sequence is from last-created resource to first-created, so that dependencies between
+ * them are respected. (There are some amongst layers in the _io module.)
+ *
+ * @param resourceClosers to be called in turn
+ */
+ private static void runClosers(Set<Callable<Void>> resourceClosers) {
+ // resourceClosers can be null in some strange cases
+ if (resourceClosers != null) {
+ /*
+ * Although a Set, the container iterates in the order closers were added. Make a Deque
+ * of it and deal from the top.
+ */
+ LinkedList<Callable<Void>> rc = new LinkedList<Callable<Void>>(resourceClosers);
+ Iterator<Callable<Void>> iter = rc.descendingIterator();
+
+ while (iter.hasNext()) {
+ Callable<Void> callable = iter.next();
+ try {
+ callable.call();
+ } catch (Exception e) {
+ // just continue, nothing we can do
+ }
+ }
+ }
+ }
+
}
diff --git a/src/org/python/modules/_io/Closer.java b/src/org/python/modules/_io/Closer.java
--- a/src/org/python/modules/_io/Closer.java
+++ b/src/org/python/modules/_io/Closer.java
@@ -55,12 +55,14 @@
*/
@Override
public synchronized Void call() {
- // This will prevent a call to dismiss() manipulating the caller's list of closers
- sys = null;
- // Call close on the client (if it still exists)
- C toClose = client.get();
- if (toClose != null) {
- toClose.invoke("close");
+ if (sys != null) {
+ // This will prevent repeated work and dismiss() manipulating the list of closers
+ sys = null;
+ // Call close on the client (if it still exists)
+ C toClose = client.get();
+ if (toClose != null) {
+ toClose.invoke("close");
+ }
}
return null;
}
diff --git a/src/org/python/modules/_io/PyFileIO.java b/src/org/python/modules/_io/PyFileIO.java
--- a/src/org/python/modules/_io/PyFileIO.java
+++ b/src/org/python/modules/_io/PyFileIO.java
@@ -168,15 +168,18 @@
throw Py.OSError(Errno.EBADF);
}
}
+ }
+ }
- } else {
- // The file was a type we don't know how to use
- throw Py.TypeError(String.format("invalid file: %s", file.__repr__().asString()));
- }
+ // If we couldn't figure it out, ioDelegate will still be null
+ if (ioDelegate == null) {
+ // The file was a type we don't know how to use
+ throw Py.TypeError(String.format("invalid file: %s", file.__repr__().asString()));
}
// The name is either the textual name or a file descriptor (see Python docs)
fastGetDict().__setitem__("name", file);
+
}
private static final String[] openArgs = {"file", "mode", "closefd"};
@@ -432,6 +435,11 @@
return PyJavaType.wrapJavaObject(ioDelegate.fileno());
}
+ @Override
+ public boolean isatty() {
+ return FileIO_isatty();
+ }
+
@ExposedMethod(doc = isatty_doc)
final boolean FileIO_isatty() {
if (__closed) {
diff --git a/src/org/python/modules/_io/_io.java b/src/org/python/modules/_io/_io.java
--- a/src/org/python/modules/_io/_io.java
+++ b/src/org/python/modules/_io/_io.java
@@ -119,76 +119,93 @@
mode.checkValid();
- int line_buffering;
-
/*
* Create the Raw file stream. Let the constructor deal with the variants and argument
* checking.
*/
PyFileIO raw = new PyFileIO(file, mode, closefd);
- // XXX Can this work: boolean isatty = raw.isatty() ? Or maybe:
- // PyObject res = PyObject_CallMethod(raw, "isatty", NULL);
- boolean isatty = false;
+ /*
+ * From the Python documentation for io.open() buffering = 0 to switch buffering off (only
+ * allowed in binary mode), 1 to select line buffering (only usable in text mode), and an
+ * integer > 1 to indicate the size of a fixed-size buffer.
+ *
+ * When no buffering argument is given, the default buffering policy works as follows:
+ * Binary files are buffered in fixed-size chunks; "Interactive" text files (files for which
+ * isatty() returns True) use line buffering. Other text files use the policy described
+ * above for binary files.
+ *
+ * In Java, it seems a stream never is *known* to be interactive, but we ask anyway, and
+ * maybe one day we shall know.
+ */
+ boolean line_buffering = false;
- /*
- * Work out a felicitous buffer size
- */
- if (buffering == 1 || (buffering < 0 && isatty)) {
- buffering = -1;
- line_buffering = 1;
- } else {
- line_buffering = 0;
- }
-
- if (buffering < 0) {
- // Try to establish the default buffer size for this file using the OS.
- buffering = _DEFAULT_BUFFER_SIZE;
- // PyObject res = PyObject_CallMethod(raw, "fileno", NULL);
- // if (fstat(fileno, &st) >= 0 && st.st_blksize > 1)
- // buffering = st.st_blksize;
- }
-
- if (buffering < 0) {
- throw Py.ValueError("invalid buffering size");
- }
-
- // If not buffering, return the raw file object
if (buffering == 0) {
if (!mode.binary) {
throw Py.ValueError("can't have unbuffered text I/O");
}
return raw;
+
+ } else if (buffering == 1) {
+ // The stream is to be read line-by-line.
+ line_buffering = true;
+ // Force default size for actual buffer
+ buffering = -1;
+
+ } else if (buffering < 0 && raw.isatty()) {
+ // No buffering argument given but stream is inteeractive.
+ line_buffering = true;
}
- // We are buffering, so wrap raw into a buffered file
- PyObject bufferType = null;
- PyObject io = imp.load("io");
-
- if (mode.updating) {
- bufferType = io.__getattr__("BufferedRandom");
- } else if (mode.writing || mode.appending) {
- bufferType = io.__getattr__("BufferedWriter");
- } else { // = reading
- bufferType = io.__getattr__("BufferedReader");
+ if (buffering < 0) {
+ /*
+ * We are still being asked for the default buffer size. CPython establishes the default
+ * buffer size using fstat(fd), but Java appears to give no clue. A useful study of
+ * buffer sizes in NIO is http://www.evanjones.ca/software/java-bytebuffers.html . This
+ * leads us to the fixed choice of _DEFAULT_BUFFER_SIZE (=8KB).
+ */
+ buffering = _DEFAULT_BUFFER_SIZE;
}
- PyInteger pyBuffering = new PyInteger(buffering);
- PyObject buffer = bufferType.__call__(raw, pyBuffering);
+ /*
+ * We now know just what particular class of file we are opening, and therefore what stack
+ * (buffering and text encoding) we should build.
+ */
+ if (buffering == 0) {
+ // Not buffering, return the raw file object
+ return raw;
- // If binary, return the buffered file
- if (mode.binary) {
- return buffer;
+ } else {
+ // We are buffering, so wrap raw into a buffered file
+ PyObject bufferType = null;
+ PyObject io = imp.load("io");
+
+ if (mode.updating) {
+ bufferType = io.__getattr__("BufferedRandom");
+ } else if (mode.writing || mode.appending) {
+ bufferType = io.__getattr__("BufferedWriter");
+ } else { // = reading
+ bufferType = io.__getattr__("BufferedReader");
+ }
+
+ PyInteger pyBuffering = new PyInteger(buffering);
+ PyObject buffer = bufferType.__call__(raw, pyBuffering);
+
+ if (mode.binary) {
+ // If binary, return the just the buffered file
+ return buffer;
+
+ } else {
+ // We are opening in text mode, so wrap buffered file in a TextIOWrapper.
+ PyObject textType = io.__getattr__("TextIOWrapper");
+ PyObject[] textArgs =
+ {buffer, ap.getPyObject(3, Py.None), ap.getPyObject(4, Py.None),
+ ap.getPyObject(5, Py.None), Py.newBoolean(line_buffering)};
+ PyObject wrapper = textType.__call__(textArgs);
+ wrapper.__setattr__("mode", new PyString(m));
+ return wrapper;
+ }
}
-
- /* We are opening in text mode, so wrap buffer into a TextIOWrapper */
- PyObject textType = io.__getattr__("TextIOWrapper");
- PyObject[] textArgs =
- {buffer, ap.getPyObject(3, Py.None), ap.getPyObject(4, Py.None),
- ap.getPyObject(5, Py.None), Py.newInteger(line_buffering)};
- PyObject wrapper = textType.__call__(textArgs);
- wrapper.__setattr__("mode", new PyString(m));
- return wrapper;
}
private static final String[] openKwds = {"file", "mode", "buffering", "encoding", "errors",
diff --git a/tests/java/org/python/modules/_io/_ioTest.java b/tests/java/org/python/modules/_io/_ioTest.java
--- a/tests/java/org/python/modules/_io/_ioTest.java
+++ b/tests/java/org/python/modules/_io/_ioTest.java
@@ -1,13 +1,24 @@
-/* Copyright (c)2012 Jython Developers */
+/* Copyright (c)2013 Jython Developers */
package org.python.modules._io;
import static org.junit.Assert.*;
import static org.junit.matchers.JUnitMatchers.*;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOError;
+import java.io.IOException;
+import java.util.Arrays;
+
import org.junit.Before;
import org.junit.Test;
+import org.python.core.Py;
import org.python.core.PyException;
+import org.python.core.PyFile;
import org.python.core.PyObject;
+import org.python.core.PyStringMap;
+import org.python.core.PySystemState;
+import org.python.core.io.RawIOBase;
import org.python.util.PythonInterpreter;
/**
@@ -18,6 +29,8 @@
public class _ioTest {
/** We need the interpreter to be initialised for these tests **/
+ PySystemState systemState;
+ PyStringMap dict;
PythonInterpreter interp;
/**
@@ -27,10 +40,10 @@
*/
@Before
public void setUp() throws Exception {
-
// Initialise a Jython interpreter
- interp = new PythonInterpreter();
-
+ systemState = Py.getSystemState();
+ dict = new PyStringMap();
+ interp = new PythonInterpreter(dict, systemState);
}
/**
@@ -81,4 +94,161 @@
}
}
+ /**
+ * Test automatic closing of files when the interpreter finally exits. Done correctly, text
+ * written to any kind of file-like object should be flushed to disk and the file closed when
+ * the PySystemState is torn down, which happens during JVM exit. We don't here (can't?) test
+ * through JVM shutdown, but we test it using the same mechanism that a JVM shutdown invokes.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void closeNeglectedFiles() throws IOException {
+
+ // File names
+ final String F = "$test_1_tmp"; // Raw file
+ final String FB = "$test_2_tmp"; // Buffered file
+ final String FT = "$test_3_tmp"; // Test file
+
+ String expText = "Line 1\nLine 2\nLine 3."; // Note: all ascii, but with new lines
+ byte[] expBytes = expText.getBytes();
+ String escapedText = expText.replace("\n", "\\n");
+
+ // The approach is to open and write files in Python, then bin the interpreter
+ interp.exec("import io\n" + //
+ "u = u'" + escapedText + "'\n" + //
+ "b = b'" + escapedText + "'\n" //
+ );
+
+ // This should get us an io.FileIO (unbuffered binary file) called f
+ interp.exec("f = io.open('" + F + "', 'wb', 0)");
+ PyIOBase pyf = (PyIOBase)interp.get("f");
+ assertNotNull(pyf);
+
+ // This should get us an io.BufferedWriter (buffered binary file) called fb
+ interp.exec("fb = io.open('" + FB + "', 'wb')");
+ PyIOBase pyfb = (PyIOBase)interp.get("fb");
+ assertNotNull(pyfb);
+
+ // This should get us an io.TextIOWrapper (buffered text file) called ft
+ interp.exec("ft = io.open('" + FT + "', 'w', encoding='ascii')");
+ PyIOBase pyft = (PyIOBase)interp.get("ft");
+ assertNotNull(pyft);
+
+ // Write the bytes test material to each file but don't close it
+ interp.exec("f.write(b)");
+ interp.exec("fb.write(b)");
+ interp.exec("ft.write(u)");
+
+ // Now bin the interpreter. (Is there a more realistic way?)
+ interp.cleanup();
+
+ // Check file itself for closure using package-visible attribute
+ assertTrue(pyf.__closed);
+ assertTrue(pyfb.__closed);
+ assertTrue(pyft.__closed);
+
+ // If they were not closed properly not all bytes will reach the files.
+ checkFileContent(F, expBytes);
+ checkFileContent(FB, expBytes);
+
+ // Expect that TextIOWrapper should have adjusted the line separator
+ checkFileContent(FT, newlineFix(expText));
+ }
+
+ /**
+ * Test automatic closing of PyFiles when the interpreter finally exits. This repeats
+ * {@link #closeNeglectedFiles()} but for the py2k flavour of file.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void closeNeglectedPyFiles() throws IOException {
+
+ final String F = "$test_1_tmp"; // Raw file
+ final String FB = "$test_2_tmp"; // Buffered file
+ final String FT = "$test_3_tmp"; // Test file
+
+ String expText = "Line 1\nLine 2\nLine 3.";
+ byte[] expBytes = expText.getBytes();
+ String escapedText = expText.replace("\n", "\\n");
+
+ // The approach is to open and write files in Python, then bin the interpreter
+ interp.exec("import io\n" + //
+ "u = u'" + escapedText + "'\n" + //
+ "b = b'" + escapedText + "'\n" //
+ );
+
+ // This should get us an unbuffered binary PyFile called f
+ interp.exec("f = open('" + F + "', 'wb', 0)");
+ PyFile pyf = (PyFile)interp.get("f");
+ assertNotNull(pyf);
+ RawIOBase r = (RawIOBase) pyf.fileno().__tojava__(RawIOBase.class);
+
+ // This should get us a buffered binary PyFile called fb
+ interp.exec("fb = open('" + FB + "', 'wb')");
+ PyFile pyfb = (PyFile)interp.get("fb");
+ assertNotNull(pyfb);
+ RawIOBase rb = (RawIOBase) pyfb.fileno().__tojava__(RawIOBase.class);
+
+ // This should get us an buffered text PyFile called ft
+ interp.exec("ft = open('" + FT + "', 'w')");
+ PyFile pyft = (PyFile)interp.get("ft");
+ assertNotNull(pyft);
+ RawIOBase rt = (RawIOBase) pyft.fileno().__tojava__(RawIOBase.class);
+
+ // Write the bytes test material to each file but don't close it
+ interp.exec("f.write(b)");
+ interp.exec("fb.write(b)");
+ interp.exec("ft.write(u)");
+
+ // Now bin the interpreter. (Is there a more realistic way?)
+ interp.cleanup();
+
+ // The PyFile itself is not closed but the underlying stream should be
+ assertTrue(r.closed());
+ assertTrue(rb.closed());
+ assertTrue(rt.closed());
+
+ // If they were not closed properly not all bytes will reach the files.
+ checkFileContent(F, expBytes);
+ checkFileContent(FB, expBytes);
+
+ // Expect that TextIOWrapper should have adjusted the line separator
+ checkFileContent(FT, newlineFix(expText));
+ }
+
+ /**
+ * Check the file contains the bytes we expect and <b>delete the file</b>. If it was not closed
+ * properly (layers in the right order and a flush) not all bytes will have reached the files.
+ *
+ * @param name of file
+ * @param expBytes expected
+ * @throws IOException if cannot open/read
+ */
+ private static void checkFileContent(String name, byte[] expBytes) throws IOException {
+ byte[] r = new byte[2 * expBytes.length];
+ File f = new File(name);
+ FileInputStream in = new FileInputStream(f);
+ int n = in.read(r);
+ in.close();
+
+ String msg = "Bytes read from " + name;
+ assertEquals(msg, expBytes.length, n);
+ byte[] resBytes = Arrays.copyOf(r, n);
+ assertArrayEquals(msg, expBytes, resBytes);
+
+ f.delete();
+ }
+
+
+ /**
+ * Replace "\n" characters by the system-defined newline sequence and return as bytes.
+ * @param expText to translate
+ * @return result as bytes
+ */
+ private static byte[] newlineFix(String expText) {
+ String newline = System.getProperty("line.separator");
+ return expText.replace("\n", newline).getBytes();
+ }
}
--
Repository URL: http://hg.python.org/jython
More information about the Jython-checkins
mailing list