[Jython-checkins] jython: Closer _io layers top to bottom.
jeff.allen
jython-checkins at python.org
Mon Jan 28 00:25:14 CET 2013
http://hg.python.org/jython/rev/68c3d85a3a6b
changeset: 6960:68c3d85a3a6b
user: Jeff Allen <ja...py at farowl.co.uk>
date: Sun Jan 27 22:23:15 2013 +0000
summary:
Closer _io layers top to bottom.
This change affects resource closing in PySstemState so that buffered files left
open by the application are flushed and closed (instead of closed and flushed).
A Java demonstrates that buffered data reaches the disk.
files:
src/org/python/core/PySystemState.java | 57 ++-
src/org/python/modules/_io/Closer.java | 14 +-
tests/java/org/python/modules/_io/_ioTest.java | 178 +++++++++-
3 files changed, 221 insertions(+), 28 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/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