[Jython-checkins] jython: Use PyBuffer try-with-resources throughout the core.

jeff.allen jython-checkins at python.org
Sat Jun 14 14:02:19 CEST 2014


http://hg.python.org/jython/rev/18d60bea34af
changeset:   7292:18d60bea34af
user:        Jeff Allen <ja.py at farowl.co.uk>
date:        Sat Jun 14 10:33:07 2014 +0100
summary:
  Use PyBuffer try-with-resources throughout the core.
Set a good example. Also, fix some places where I forgot to release().
Similar work needed on binascii, but defer to a separate patch.

files:
  src/org/python/core/BaseBytes.java            |  110 +-----
  src/org/python/core/PyArray.java              |    5 +-
  src/org/python/core/PyByteArray.java          |  147 +++++----
  src/org/python/core/PyFile.java               |    6 +-
  src/org/python/core/PyString.java             |    5 +-
  src/org/python/core/PyUnicode.java            |    5 +-
  src/org/python/core/io/TextIOBase.java        |   16 +-
  src/org/python/modules/sre/PatternObject.java |    6 +-
  8 files changed, 119 insertions(+), 181 deletions(-)


diff --git a/src/org/python/core/BaseBytes.java b/src/org/python/core/BaseBytes.java
--- a/src/org/python/core/BaseBytes.java
+++ b/src/org/python/core/BaseBytes.java
@@ -345,10 +345,11 @@
      */
     protected void init(BufferProtocol value) throws PyException {
         // Get the buffer view
-        PyBuffer view = value.getBuffer(PyBUF.FULL_RO);
-        // Create storage for the bytes and have the view drop them in
-        newStorage(view.getLen());
-        view.copyTo(storage, offset);
+        try (PyBuffer view = value.getBuffer(PyBUF.FULL_RO)) {
+            // Create storage for the bytes and have the view drop them in
+            newStorage(view.getLen());
+            view.copyTo(storage, offset);
+        }
     }
 
     /**
@@ -834,21 +835,16 @@
             return 0;
 
         } else {
-
             // Try to get a byte-oriented view
-            PyBuffer bv = getView(b);
-
-            if (bv == null) {
-                // Signifies a type mis-match. See PyObject._cmp_unsafe() and related code.
-                return -2;
-
-            } else {
-                try {
+            try (PyBuffer bv = getView(b)) {
+
+                if (bv == null) {
+                    // Signifies a type mis-match. See PyObject._cmp_unsafe() and related code.
+                    return -2;
+
+                } else {
                     // Compare this with other object viewed as a buffer
                     return compare(this, bv);
-                } finally {
-                    // Must alsways let go of the buffer
-                    bv.release();
                 }
             }
         }
@@ -871,14 +867,13 @@
         } else {
 
             // Try to get a byte-oriented view
-            PyBuffer bv = getView(b);
-
-            if (bv == null) {
-                // Signifies a type mis-match. See PyObject._cmp_unsafe() and related code.
-                return -2;
-
-            } else {
-                try {
+            try (PyBuffer bv = getView(b)) {
+
+                if (bv == null) {
+                    // Signifies a type mis-match. See PyObject._cmp_unsafe() and related code.
+                    return -2;
+
+                } else {
                     if (bv.getLen() != size) {
                         // Different size: can't be equal, and we don't care which is bigger
                         return 1;
@@ -886,9 +881,6 @@
                         // Compare this with other object viewed as a buffer
                         return compare(this, bv);
                     }
-                } finally {
-                    // Must alsways let go of the buffer
-                    bv.release();
                 }
             }
         }
@@ -1017,13 +1009,10 @@
             return index(b) >= 0;
         } else {
             // Caller is treating this as a byte-string and looking for substring 'target'
-            PyBuffer targetView = getViewOrError(target);
-            try {
+            try (PyBuffer targetView = getViewOrError(target)) {
                 Finder finder = new Finder(targetView);
                 finder.setText(this);
                 return finder.nextIndex() >= 0;
-            } finally {
-                targetView.release();
             }
         }
     }
@@ -1080,9 +1069,7 @@
     private boolean match(PyObject target, int pos, int n, boolean endswith) {
 
         // Error if not something we can treat as a view of bytes
-        PyBuffer vt = getViewOrError(target);
-
-        try {
+        try (PyBuffer vt = getViewOrError(target)) {
             int j = 0, len = vt.getLen();
 
             if (!endswith) {
@@ -1106,10 +1093,6 @@
                 }
             }
             return true; // They must all have matched
-
-        } finally {
-            // Let go of the buffer we acquired
-            vt.release();
         }
     }
 
@@ -1818,8 +1801,7 @@
      * @return count of occurrences of sub within this byte array
      */
     final int basebytes_count(PyObject sub, PyObject ostart, PyObject oend) {
-        PyBuffer vsub = getViewOrError(sub);
-        try {
+        try (PyBuffer vsub = getViewOrError(sub)) {
             Finder finder = new Finder(vsub);
 
             // Convert [ostart:oend] to integers
@@ -1827,8 +1809,6 @@
 
             // Make this slice the thing we count within.
             return finder.count(storage, offset + index[0], index[3]);
-        } finally {
-            vsub.release();
         }
     }
 
@@ -1846,12 +1826,9 @@
      * @return index of start of occurrence of sub within this byte array
      */
     final int basebytes_find(PyObject sub, PyObject ostart, PyObject oend) {
-        PyBuffer vsub = getViewOrError(sub);
-        try {
+        try (PyBuffer vsub = getViewOrError(sub)) {
             Finder finder = new Finder(vsub);
             return find(finder, ostart, oend);
-        } finally {
-            vsub.release();
         }
     }
 
@@ -2026,9 +2003,7 @@
     final synchronized PyTuple basebytes_partition(PyObject sep) {
 
         // View the separator as a byte array (or error if we can't)
-        PyBuffer separator = getViewOrError(sep);
-
-        try {
+        try (PyBuffer separator = getViewOrError(sep)) {
             // Create a Finder for the separator and set it on this byte array
             int n = checkForEmptySeparator(separator);
             Finder finder = new Finder(separator);
@@ -2043,8 +2018,6 @@
                 // Not found: choose values leading to ([0:size], '', '')
                 return partition(size, size);
             }
-        } finally {
-            separator.release();
         }
     }
 
@@ -2080,12 +2053,9 @@
      * @return index of start of occurrence of sub within this byte array
      */
     final int basebytes_rfind(PyObject sub, PyObject ostart, PyObject oend) {
-        PyBuffer vsub = getViewOrError(sub);
-        try {
+        try (PyBuffer vsub = getViewOrError(sub)) {
             Finder finder = new ReverseFinder(vsub);
             return find(finder, ostart, oend);
-        } finally {
-            vsub.release();
         }
     }
 
@@ -2128,9 +2098,7 @@
     final synchronized PyByteArray basebytes_replace(PyObject oldB, PyObject newB, int maxcount) {
 
         // View the to and from as byte arrays (or error if we can't)
-        PyBuffer to = getViewOrError(newB), from = null;
-        try {
-            from = getViewOrError(oldB);
+        try (PyBuffer to = getViewOrError(newB); PyBuffer from = getViewOrError(oldB)) {
             /*
              * The logic of the first section is copied exactly from CPython in order to get the
              * same behaviour. The "headline" description of replace is simple enough but the corner
@@ -2184,16 +2152,6 @@
                 // Otherwise use the generic algorithm
                 return replace_substring(from, to, maxcount);
             }
-
-        } finally {
-            /*
-             * Release the buffers we acquired: there must be a to buffer and there might be a from
-             * buffer.
-             */
-            to.release();
-            if (from != null) {
-                from.release();
-            }
         }
     }
 
@@ -2475,8 +2433,7 @@
     final synchronized PyTuple basebytes_rpartition(PyObject sep) {
 
         // View the separator as a byte array (or error if we can't)
-        PyBuffer separator = getViewOrError(sep);
-        try {
+        try (PyBuffer separator = getViewOrError(sep)) {
             // Create a Finder for the separtor and set it on this byte array
             int n = checkForEmptySeparator(separator);
             Finder finder = new ReverseFinder(separator);
@@ -2491,8 +2448,6 @@
                 // Not found: choose values leading to ('', '', [0:size])
                 return partition(0, 0);
             }
-        } finally {
-            separator.release();
         }
     }
 
@@ -2590,9 +2545,7 @@
     final synchronized PyList basebytes_rsplit_explicit(PyObject sep, int maxsplit) {
 
         // The separator may be presented as anything viewable as bytes
-        PyBuffer separator = getViewOrError(sep);
-
-        try {
+        try (PyBuffer separator = getViewOrError(sep)) {
             int n = checkForEmptySeparator(separator);
 
             PyList result = new PyList();
@@ -2627,8 +2580,6 @@
                 result.add(0, word);
             }
             return result;
-        } finally {
-            separator.release();
         }
     }
 
@@ -2794,8 +2745,7 @@
     final synchronized PyList basebytes_split_explicit(PyObject sep, int maxsplit) {
 
         // The separator may be presented as anything viewable as bytes
-        PyBuffer separator = getViewOrError(sep);
-        try {
+        try (PyBuffer separator = getViewOrError(sep)) {
             checkForEmptySeparator(separator);
 
             PyList result = new PyList();
@@ -2821,8 +2771,6 @@
             // Append the remaining unsplit text
             result.append(getslice(p - offset, size));
             return result;
-        } finally {
-            separator.release();
         }
     }
 
diff --git a/src/org/python/core/PyArray.java b/src/org/python/core/PyArray.java
--- a/src/org/python/core/PyArray.java
+++ b/src/org/python/core/PyArray.java
@@ -1131,8 +1131,7 @@
 
             } else {
                 // Access the bytes
-                PyBuffer pybuf = ((BufferProtocol)input).getBuffer(PyBUF.STRIDED_RO);
-                try {
+                try (PyBuffer pybuf = ((BufferProtocol)input).getBuffer(PyBUF.STRIDED_RO)) {
                     // Provide argument as stream of bytes for fromstream method
                     if (pybuf.getNdim() == 1) {
                         if (pybuf.getStrides()[0] == 1) {
@@ -1149,8 +1148,6 @@
                         // Currently don't support n-dimensional sources
                         throw Py.ValueError("multi-dimensional buffer not supported");
                     }
-                } finally {
-                    pybuf.release();
                 }
             }
 
diff --git a/src/org/python/core/PyByteArray.java b/src/org/python/core/PyByteArray.java
--- a/src/org/python/core/PyByteArray.java
+++ b/src/org/python/core/PyByteArray.java
@@ -548,24 +548,25 @@
      */
     private void setslice(int start, int stop, int step, BufferProtocol value) throws PyException {
 
-        PyBuffer view = value.getBuffer(PyBUF.FULL_RO);
+        try (PyBuffer view = value.getBuffer(PyBUF.FULL_RO)) {
 
-        int len = view.getLen();
+            int len = view.getLen();
 
-        if (step == 1) {
-            // Delete this[start:stop] and open a space of the right size
-            storageReplace(start, stop - start, len);
-            view.copyTo(storage, start + offset);
+            if (step == 1) {
+                // Delete this[start:stop] and open a space of the right size
+                storageReplace(start, stop - start, len);
+                view.copyTo(storage, start + offset);
 
-        } else {
-            // This is an extended slice which means we are replacing elements
-            int n = sliceLength(start, stop, step);
-            if (n != len) {
-                throw SliceSizeError("bytes", len, n);
-            }
+            } else {
+                // This is an extended slice which means we are replacing elements
+                int n = sliceLength(start, stop, step);
+                if (n != len) {
+                    throw SliceSizeError("bytes", len, n);
+                }
 
-            for (int io = start + offset, j = 0; j < n; io += step, j++) {
-                storage[io] = view.byteAt(j);    // Assign this[i] = value[j]
+                for (int io = start + offset, j = 0; j < n; io += step, j++) {
+                    storage[io] = view.byteAt(j);    // Assign this[i] = value[j]
+                }
             }
         }
     }
@@ -2021,66 +2022,78 @@
     @ExposedMethod(defaults = "null", doc = BuiltinDocs.bytearray_translate_doc)
     final PyByteArray bytearray_translate(PyObject table, PyObject deletechars) {
 
-        // Normalise the translation table to a View
+        // Work with the translation table (if there is one) as a PyBuffer view.
+        try (PyBuffer tab = getTranslationTable(table)) {
+
+            // Accumulate the result here
+            PyByteArray result = new PyByteArray();
+
+            // There are 4 cases depending on presence/absence of table and deletechars
+
+            if (deletechars != null) {
+
+                // Work with the deletion characters as a buffer too.
+                try (PyBuffer d = getViewOrError(deletechars)) {
+                    // Use a ByteSet to express which bytes to delete
+                    ByteSet del = new ByteSet(d);
+                    int limit = offset + size;
+                    if (tab == null) {
+                        // No translation table, so we're just copying with omissions
+                        for (int i = offset; i < limit; i++) {
+                            int b = storage[i] & 0xff;
+                            if (!del.contains(b)) {
+                                result.append((byte)b);
+                            }
+                        }
+                    } else {
+                        // Loop over this byte array and write translated bytes to the result
+                        for (int i = offset; i < limit; i++) {
+                            int b = storage[i] & 0xff;
+                            if (!del.contains(b)) {
+                                result.append(tab.byteAt(b));
+                            }
+                        }
+                    }
+                }
+
+            } else {
+                // No deletion set.
+                if (tab == null) {
+                    // ... and no translation table either: just copy
+                    result.extend(this);
+                } else {
+                    int limit = offset + size;
+                    // Loop over this byte array and write translated bytes to the result
+                    for (int i = offset; i < limit; i++) {
+                        int b = storage[i] & 0xff;
+                        result.append(tab.byteAt(b));
+                    }
+                }
+            }
+
+            return result;
+        }
+    }
+
+    /**
+     * Return a {@link PyBuffer} representing a translation table, or raise an exception if it is
+     * the wrong size. The caller is responsible for calling {@link PyBuffer#release()} on any
+     * returned buffer.
+     *
+     * @param table the translation table (or <code>null</code> or {@link PyNone})
+     * @return the buffer view of the table or null if there is no table
+     * @throws PyException if the table is not exacltly 256 bytes long
+     */
+    private PyBuffer getTranslationTable(PyObject table) throws PyException {
         PyBuffer tab = null;
+        // Normalise the translation table to a View (if there is one).
         if (table != null && table != Py.None) {
             tab = getViewOrError(table);
             if (tab.getLen() != 256) {
                 throw Py.ValueError("translation table must be 256 bytes long");
             }
         }
-
-        // Accumulate the result here
-        PyByteArray result = new PyByteArray();
-
-        // There are 4 cases depending on presence/absence of table and deletechars
-
-        if (deletechars != null) {
-
-            // Use a ByteSet to express which bytes to delete
-            ByteSet del;
-            del = new ByteSet(getViewOrError(deletechars));
-
-            // Now, loop over this byte array and write translated bytes to the result
-            int limit = offset + size;
-
-            if (tab != null) {
-                for (int i = offset; i < limit; i++) {
-                    int b = storage[i] & 0xff;
-                    if (!del.contains(b)) {
-                        result.append(tab.byteAt(b));
-                    }
-                }
-
-            } else {
-                // No translation table
-                for (int i = offset; i < limit; i++) {
-                    int b = storage[i] & 0xff;
-                    if (!del.contains(b)) {
-                        result.append((byte)b);
-                    }
-                }
-            }
-
-        } else {
-            // No deletion set.
-
-            // Now, loop over this byte array and write translated bytes to the result
-            int limit = offset + size;
-            if (tab != null) {
-                for (int i = offset; i < limit; i++) {
-                    int b = storage[i] & 0xff;
-                    result.append(tab.byteAt(b));
-                }
-
-            } else {
-                // No translation table or deletion set: just copy
-                result.extend(this);
-            }
-
-        }
-
-        return result;
+        return tab;
     }
 
     /**
diff --git a/src/org/python/core/PyFile.java b/src/org/python/core/PyFile.java
--- a/src/org/python/core/PyFile.java
+++ b/src/org/python/core/PyFile.java
@@ -478,13 +478,9 @@
 
         } else if (obj instanceof BufferProtocol) {
             // Try to get a byte-oriented buffer
-            PyBuffer buf = ((BufferProtocol)obj).getBuffer(PyBUF.FULL_RO);
-            try {
+            try (PyBuffer buf = ((BufferProtocol)obj).getBuffer(PyBUF.FULL_RO)) {
                 // ... and treat those bytes as a String
                 return buf.toString();
-            } finally {
-                // We should release the buffer
-                buf.release();
             }
         }
 
diff --git a/src/org/python/core/PyString.java b/src/org/python/core/PyString.java
--- a/src/org/python/core/PyString.java
+++ b/src/org/python/core/PyString.java
@@ -684,11 +684,8 @@
             return ((PyString)obj).getString();
         } else if (obj instanceof BufferProtocol) {
             // Other object with buffer API: briefly access the buffer
-            PyBuffer buf = ((BufferProtocol)obj).getBuffer(PyBUF.SIMPLE);
-            try {
+            try (PyBuffer buf = ((BufferProtocol)obj).getBuffer(PyBUF.FULL_RO)) {
                 return buf.toString();
-            } finally {
-                buf.release();
             }
         } else {
             return null;
diff --git a/src/org/python/core/PyUnicode.java b/src/org/python/core/PyUnicode.java
--- a/src/org/python/core/PyUnicode.java
+++ b/src/org/python/core/PyUnicode.java
@@ -443,11 +443,8 @@
             return (PyUnicode)o;
         } else if (o instanceof BufferProtocol) {
             // PyString or PyByteArray, PyMemoryView, Py2kBuffer ...
-            PyBuffer buf = ((BufferProtocol)o).getBuffer(PyBUF.FULL_RO);
-            try {
+            try (PyBuffer buf = ((BufferProtocol)o).getBuffer(PyBUF.FULL_RO)) {
                 return new PyUnicode(buf.toString());
-            } finally {
-                buf.release();
             }
         } else {
             // o is some type not allowed:
diff --git a/src/org/python/core/io/TextIOBase.java b/src/org/python/core/io/TextIOBase.java
--- a/src/org/python/core/io/TextIOBase.java
+++ b/src/org/python/core/io/TextIOBase.java
@@ -115,13 +115,11 @@
             return read.length();
 
         } else if (buf instanceof BufferProtocol) {
-            PyBuffer view = ((BufferProtocol)buf).getBuffer(PyBUF.SIMPLE);
-            if (view.isReadonly()) {
-                // More helpful than falling through to CPython message
-                throw Py.TypeError("cannot read into read-only " + buf.getType().fastGetName());
-
-            } else {
-                try {
+            try (PyBuffer view = ((BufferProtocol)buf).getBuffer(PyBUF.FULL_RO)) {
+                if (view.isReadonly()) {
+                    // More helpful than falling through to CPython message
+                    throw Py.TypeError("cannot read into read-only " + buf.getType().fastGetName());
+                } else {
                     // Inefficiently, we have to go via a String
                     String read = read(view.getLen());
                     int n = read.length();
@@ -129,11 +127,7 @@
                         view.storeAt((byte)read.charAt(i), i);
                     }
                     return read.length();
-                } finally {
-                    // We should release the buffer explicitly
-                    view.release();
                 }
-
             }
         }
 
diff --git a/src/org/python/modules/sre/PatternObject.java b/src/org/python/modules/sre/PatternObject.java
--- a/src/org/python/modules/sre/PatternObject.java
+++ b/src/org/python/modules/sre/PatternObject.java
@@ -373,13 +373,9 @@
 
         } else if (obj instanceof BufferProtocol) {
             // Try to get a byte-oriented buffer
-            PyBuffer buf = ((BufferProtocol)obj).getBuffer(PyBUF.FULL_RO);
-            try {
+            try (PyBuffer buf = ((BufferProtocol)obj).getBuffer(PyBUF.FULL_RO)){
                 // ... and treat those bytes as a PyString
                 return new PyString(buf.toString());
-            } finally {
-                // We should release the buffer
-                buf.release();
             }
         }
 

-- 
Repository URL: http://hg.python.org/jython


More information about the Jython-checkins mailing list